Skip to content

Provider serves DNS proxy on Openvpn's virtual IP - #1281

Merged
Waldz merged 4 commits into
masterfrom
feature/dns-proxy-proc
Aug 29, 2019
Merged

Provider serves DNS proxy on Openvpn's virtual IP#1281
Waldz merged 4 commits into
masterfrom
feature/dns-proxy-proc

Conversation

@Waldz

@Waldz Waldz commented Aug 27, 2019

Copy link
Copy Markdown
Member

Screen Shot 2019-08-27 at 15 19 31

@Waldz
Waldz requested a review from tadovas as a code owner August 27, 2019 12:20
@Waldz
Waldz force-pushed the feature/dns-proxy-proc branch from cc624a7 to f3c17fd Compare August 27, 2019 12:26
Comment thread services/openvpn/service/manager.go Outdated
@Waldz
Waldz force-pushed the feature/dns-proxy-proc branch from f3c17fd to e5f58d2 Compare August 29, 2019 10:09
@Waldz
Waldz requested a review from soffokl August 29, 2019 10:09
m.natPinger.BindServicePort(openvpn_service.ServiceType, m.vpnServerPort)

log.Info(logPrefix, "starting openvpn server on port: ", m.vpnServerPort)
log.Info(logPrefix, "Starting openvpn server on port: ", m.vpnServerPort)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case should be consistent, i.e. lowercase

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So logs most places we do with log.Info("Uppercase ..").
Maybe You're mixing with errors.New("lowercase ..") which is Golang convention.

@tadaskay tadaskay Aug 29, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No @Waldz, I'm talking about the logging messages. They are not consistent throughout the application, thus no, in most places, we don't do capitalization of the log messages. Quite often, the error is being logged as-is, resulting in a lowercase message as well. Perhaps it's a good addition for our devbook if we can agree on the choice?

@Waldz
Waldz merged commit 3ebe026 into master Aug 29, 2019
@Waldz
Waldz deleted the feature/dns-proxy-proc branch August 29, 2019 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants