-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor!: Use RepositoryPermissions struct for User.Permissions
#3963
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
refactor!: Use RepositoryPermissions struct for User.Permissions
#3963
Conversation
Replace map[string]bool with *RepositoryPermissions struct for the User.Permissions field, providing better type safety and consistency with the existing Repository.Permissions field. The permissions object has a well-defined structure according to the GitHub API docs, making a struct more appropriate than a dynamic map. Fixes google#3947
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
RepositoryPermissions struct for User.Permissions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3963 +/- ##
=======================================
Coverage 85.51% 85.51%
=======================================
Files 204 204
Lines 17554 17554
=======================================
Hits 15011 15011
Misses 1913 1913
Partials 630 630 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
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.
Thank you, @niveshdandyan!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra
Not-Dhananjay-Mishra
left a comment
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
|
Thank you, @Not-Dhananjay-Mishra! |
BREAKING CHANGE:
User.Permissionsis now*RepositoryPermissionsinstead ofmap[string]bool.Summary
Replace
map[string]boolwith*RepositoryPermissionsstruct for theUser.Permissionsfield.Fixes #3947
Changes
User.Permissionsfrommap[string]boolto*RepositoryPermissionsRationale
According to the List Collaborators API docs, the permissions object has a fixed, well-defined structure with
pull,triage,push,maintain, andadminboolean fields.Using the existing
RepositoryPermissionsstruct provides:Repository.PermissionsBreaking Change
This is a breaking API change as noted in the issue labels. Users who access
User.Permissionswill need to update their code from map access (u.Permissions["pull"]) to struct access (u.GetPermissions().GetPull()).Testing
All existing tests pass with the updated types.