-
Notifications
You must be signed in to change notification settings - Fork 9.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
server/embed: address golangci var-naming issues #17820
server/embed: address golangci var-naming issues #17820
Conversation
Addresses issues in ListenPeerUrls, ListenClientUrls, ListenClientHttpUrls, AdvertisePeerUrls, AdvertiseClientUrls. Signed-off-by: Ivan Valdes <[email protected]>
ListenPeerUrls string `json:"listen-peer-urls"` | ||
ListenClientUrls string `json:"listen-client-urls"` | ||
ListenClientHttpUrls string `json:"listen-client-http-urls"` | ||
AdvertisePeerUrls string `json:"initial-advertise-peer-urls"` | ||
AdvertiseClientUrls string `json:"advertise-client-urls"` | ||
ListenPeerURLs string `json:"listen-peer-urls"` | ||
ListenClientURLs string `json:"listen-client-urls"` | ||
ListenClientHTTPURLs string `json:"listen-client-http-urls"` | ||
AdvertisePeerURLs string `json:"initial-advertise-peer-urls"` | ||
AdvertiseClientURLs string `json:"advertise-client-urls"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stated in the PR description, these are exported fields from an unexported struct. It's internally used to marshal/unmarshal JSONs. So, it should be fine to rename.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right
ListenPeerURLs string `json:"listen-peer-urls"` | ||
ListenClientURLs string `json:"listen-client-urls"` | ||
ListenClientHTTPURLs string `json:"listen-client-http-urls"` | ||
AdvertiseClientURLs string `json:"advertise-client-urls"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stated in the PR description, these are used only for tests marshaling the config into JSON, so it should be safe to rename them.
/retest |
Addresses issues in ListenPeerUrls, ListenClientUrls, ListenClientHttpUrls, AdvertisePeerUrls, AdvertiseClientUrls.
Following the same approach as in #17674, a linter ignore is added to these exported fields. However, non-exported functions and exported struct fields used only for JSON marshaling can be safely renamed.
Part of #17578.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.