Skip to content
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

client: address golangci var-naming issues #17582

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Mar 13, 2024

If we need to deprecate rather than rename an exported variable, we can add an ignore to the linter for that line.

Renamed exported variable:

  • logutil.JsonLogFormat -> logutil.JSONLogFormat

Renamed exported variables from internal packages:

  • endpoint.CREDS_REQUIRE -> endpoint.CredsRequire
  • endpoint.CREDS_DROP -> endpoint.CredsDrop
  • endpoint.CREDS_OPTIONAL -> endpoint.CredsOptional

Exported renamed variables to unexported as they weren't being used anywhere outside their package:

  • verify.ENV_VERIFY -> verify.envVerify
  • verify.ENV_VERIFY_VALUE_ALL -> verify.envVerifyValueAll
  • verify.ENV_VERIFY_VALUE_ASSERT -> verify.envVerifyValueAssert

After addressing these, the var-naming rule passes in the client directory.

Relates to #17578

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ivanvc ivanvc mentioned this pull request Mar 13, 2024
20 tasks
@ivanvc
Copy link
Member Author

ivanvc commented Mar 13, 2024

/retest

@ahrtr
Copy link
Member

ahrtr commented Mar 13, 2024

/ok-to-test

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks for raising these as smaller pr's for each package 🙏🏻

@@ -17,19 +17,19 @@ package logutil
import "fmt"

const (
JsonLogFormat = "json"
Copy link
Member

Choose a reason for hiding this comment

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

The only concern is this is a public/exported const. Probably we should

  • deprecate the existing name and plan to remove it in next major or minor release and skip the linter for this const;
  • add a new const JSONLogFormat.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, I deprecated JsonLogFormat.

@ivanvc ivanvc force-pushed the address-client-var-naming-lint-rule branch from 2607ad0 to 578b784 Compare March 15, 2024 03:47
@ahrtr ahrtr merged commit d639abe into etcd-io:main Mar 15, 2024
39 checks passed
@ivanvc ivanvc deleted the address-client-var-naming-lint-rule branch March 15, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants