-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add permissions to teams #1864
Add permissions to teams #1864
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
Also, please make sure to run |
You will want to commit/push (not force-push) the changes to the PR once you run |
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Hmmm... it looks like my |
I got caught up on Stringify returning a string with the empty Permissions{} in it when Team is converted to a string. This was easy to fix in strings_test, but the auto-generated github-stringify tests for It seems like the best solution might be to have the stringifyValue function in strings.go omit empty Permissions. |
I've reverted the pointer change to get tests passing — happy to change back if we'd like to change the stringify methods |
Codecov Report
@@ Coverage Diff @@
## master #1864 +/- ##
=======================================
Coverage 97.65% 97.65%
=======================================
Files 105 105
Lines 6769 6769
=======================================
Hits 6610 6610
Misses 86 86
Partials 73 73
Continue to review full report at Codecov.
|
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, @derekjobst !
LGTM.
Merging.
@gmlewis do you know when we might be able to get this into a release? It looks like there are a few other changes since the last one :) |
This PR addresses #1863 by adding a Permissions field to the Team type.