-
Notifications
You must be signed in to change notification settings - Fork 13
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
make a enum gen workflow #265
Conversation
Codecov Report
@@ Coverage Diff @@
## main #265 +/- ##
==========================================
- Coverage 77.77% 77.45% -0.32%
==========================================
Files 50 50
Lines 3275 3335 +60
==========================================
+ Hits 2547 2583 +36
- Misses 531 549 +18
- Partials 197 203 +6
|
@@ -1,28 +1,28 @@ | |||
package opslevel | |||
|
|||
type ServiceOwnershipCheckFragment struct { | |||
RequireContactMethod *bool `graphql:"requireContactMethod"` | |||
ContactMethod *ServiceOwnershipCheckContactType `graphql:"contactMethod"` |
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.
enum.go doesn't generate this enum and it seems like contact type is the same thing so consolidating this.
"net/http" | ||
"strings" | ||
|
||
"github.com/hashicorp/go-retryablehttp" | ||
"github.com/hasura/go-graphql-client" |
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.
gofumpt changes that fail go vet
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.
The test
github action related to this includes running task lint
defined in the Taskfile. Specifically it runs gofumpt -d .
with the -d
flag printing a diff - gofumpt itself doesn't write any changes.
The failure is from go vet ./...
Error: vet: ./infra_test.go:288:3: too few values in struct literal of type struct{Request string; Variables string; Response string}
caused by some changes made in a recently merge PR
@@ -298,27 +298,6 @@ var AllContactType = []string{ | |||
string(ContactTypeGitHub), | |||
} | |||
|
|||
// TODO: This appears to be duplicative of the above and i'm not sure why we need it |
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.
basically - actioning this TODO - so that we can have enum.go be auto generated at the click of the button
@@ -460,6 +439,7 @@ const ( | |||
PredicateKeyEnumTags PredicateKeyEnum = "tags" // Filter by `tags` field. | |||
PredicateKeyEnumOwnerID PredicateKeyEnum = "owner_id" // Filter by `owner` field. | |||
PredicateKeyEnumGroupIDs PredicateKeyEnum = "group_ids" // Filter by group hierarchy. Will return resources who's owner is in the group ancestry chain. | |||
PredicateKeyEnumOwnerIDs PredicateKeyEnum = "owner_ids" // Filter by `owner` hierarchy. Will return resources who's owner is in the team ancestry chain. |
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.
hey look a new enum value 🎉
UserRoleAdmin UserRole = "admin" // An administrator on the account. | ||
UserRoleUser UserRole = "user" // A regular user on the account. | ||
UserRoleAdmin UserRole = "admin" // An administrator on the account. | ||
UserRoleBasicUser UserRole = "basic_user" // A basic user on the account with limited access. |
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.
This is due to the new RBAC feature flag. I'm told that if a customer tries to use this when their feature flag is not on they will get an error message saying its not enabled on their account. So this should be safe
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.
LGTM
Changelog
Tophatting
Have not tested this yet