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

Use string constants consistently #163

Closed
willbeason opened this issue Nov 30, 2021 · 6 comments
Closed

Use string constants consistently #163

willbeason opened this issue Nov 30, 2021 · 6 comments
Assignees

Comments

@willbeason
Copy link
Member

Strings like the following should live in their canonical locations in kubernetes packages:

"v1alpha1"
"v1beta1"
"constraints.gatekeeper.sh"

This aids discoverability, makes testing easier, and ensures that differences in manually-typed strings are impossible.

@willbeason
Copy link
Member Author

willbeason commented Dec 14, 2021

goconst is a great tool for discovering these. Don't worry about getting every instance - just the ones that make sense/are important to flow.

go get github.com/jgautheron/goconst/cmd/goconst

goconst -ignore "remote" ./pkg/... | sort

Anything in client/drivers/remote is safe to ignore

@willbeason
Copy link
Member Author

The ones I've found are:

  • "v1"
  • "v1alpha1"
  • "v1beta1"
  • "constraints.gatekeeper.sh"
  • "templates.gatekeeper.sh"
  • "externaldata.gatekeeper.sh"

For the /api/ locations, the appropriate place is in a declaration like:

	SchemeGroupVersion = schema.GroupVersion{Group: "externaldata.gatekeeper.sh", Version: "v1alpha1"}

In some cases, we define a constant but the code doesn't use it. For example "ProviderRequest" has a definition in constraint/pkg/externaldata/request.go but it isn't referenced in the same file.

"data.lib" and "data.inventory" aren't repeated a bunch of times, but they're so important we should have them as constants so it's clear that their repetition in both "main.go" and "client.go" is meaningful and can't accidentally be broken.

That's everything I could find. There might be more (feel free to look if you want).

@willbeason
Copy link
Member Author

Also "data" since we define it as the root object users reference, so it pops up in a bunch of locations.

@maxsmythe
Copy link
Contributor

Is this a blocker for sharding?

@becky-hd
Copy link
Contributor

Moving constants to pkg/apis seems reasonable

@willbeason
Copy link
Member Author

Mostly done as parts of various PRs.

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

No branches or pull requests

3 participants