-
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 support to list custom roles for organizations #2336
Conversation
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, @tamboliasir1 !
Let's please change some of the naming and update the comments.
Please also make sure to run |
Hi @gmlewis I have made changes and pushed commit. Can you please review? Thank you so much. |
Codecov Report
@@ Coverage Diff @@
## master #2336 +/- ##
=======================================
Coverage 98.04% 98.05%
=======================================
Files 118 119 +1
Lines 10458 10471 +13
=======================================
+ Hits 10254 10267 +13
Misses 140 140
Partials 64 64
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.
Sorry, I was wrong.
After you fix the URL in the comments, LGTM.
We will then be ready for a second LGTM+Approval from any other contributor to this repo before merging.
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.
Please fix the typos. :-)
HI @reedloden @gmlewis Sorry for this silly typo I have resolved it please review. Please let me know if there is anything. Thank you. |
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, @tamboliasir1 !
And thank you, @reedloden for catching the typos!
LGTM.
Awaiting second LGTM from any other contributor to this repo before merging.
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 (Not a previous contributor though, just an interested party 😄 )
Thank you, @joshuahancox ! Merging. |
Thank you @gmlewis. Would love to contribute more!!😄 |
@gmlewis can we get a release of |
I will work on getting a release out before Saturday. |
@reedloden - https://github.com/google/go-github/releases/tag/v44.0.0 is now available that contains this PR. |
@gmlewis woot woot. Thank you so much! |
Fixes: #2327 .
Thanks for this opportunity.