-
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
feat: Add get for custom org repo role #3372
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3372 +/- ##
==========================================
- Coverage 97.72% 92.30% -5.42%
==========================================
Files 153 176 +23
Lines 13390 15044 +1654
==========================================
+ Hits 13085 13887 +802
- Misses 215 1064 +849
- Partials 90 93 +3 ☔ View full report in Codecov by Sentry. |
e10a8d7
to
a5c855e
Compare
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, @stevehipwell !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
(Please use regular push (not force-push) in this repo because we always use "squash & merge" anyway and it is much easier for reviewers to see what has changed since the last look... thank you!)
RE force push, I'll try and do that next time but it's muscle memory at this point so apologies in advance. I think the desktop GH interface now shows diffs for force push events, but I'm currently on mobile so can't check. |
a5c855e
to
f1c2f78
Compare
@gmlewis I've rebased this PR, is there anything outstanding on it? |
Only a second LGTM+Approval from any other contributor to this repo. |
@gmlewis what is the requirement for the second approval? Someone who's already contributed or one of the maintainers or something else? |
We have never put any restrictions on this. Our official thoughts are detailed in CONTRIBUTING.md. In my personal opinion, we welcome contributions from anyone who is interested in the Go programming language. |
@gmlewis my question above was about where the second set of eyes to approve a PR needs to come from? Do you need another maintainer or just another set of eyes from somewhere? |
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
f1c2f78
to
03073d5
Compare
I apologize that my answer was not clear, as I was attempting to answer this question. |
@stevehipwell - can you please make a concerted effort to stop force-pushing in this repo, as now I must re-read the whole PR because I cannot see what changed since the last review. |
@gmlewis I'm just rebasing onto the min branch, my code isn't changing and GitHub shouldn't be clearing your approval. I can stop doing that, but you'll need to do it before the automation runs and the PR is merged to get a linear commit history (and in some cases GitHub can't do it in the UI). |
Actually, this isn't necessary because we always use "Squash and merge" in this repo, so the commit history is always clean. |
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, @tomfeigin ! |
Resolves #3370
This PR adds the missing
GetCustomRepoRole
function.