Skip to content
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

CI: Skip 'required reviewers' check on forks (no org team check perms) #36370

Merged
merged 1 commit into from Apr 23, 2024

Conversation

aaronsteers
Copy link
Collaborator

@aaronsteers aaronsteers commented Mar 21, 2024

This was cherry-picked from the PR:

This skips membership checks when on a fork, where don't have permissions to do so.

(Set to auto-merge on approval.)

@aaronsteers aaronsteers requested a review from a team as a code owner March 21, 2024 21:27
Copy link

vercel bot commented Mar 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Mar 21, 2024 9:27pm

@aaronsteers aaronsteers changed the title Skip review-requirements check on forks (no org team check perms) CI: Skip review-requirements check on forks (no org team check perms) Mar 21, 2024
@aaronsteers aaronsteers changed the title CI: Skip review-requirements check on forks (no org team check perms) CI: Skip 'required reviewers' check on forks (no org team check perms) Mar 21, 2024
@aaronsteers aaronsteers enabled auto-merge (squash) March 21, 2024 21:33
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be cleaner to discard the use of ${{ secrets.OCTAVIA_4_ROOT_ACCESS}} and replace it with ${{ GITHUB_TOKEN}}
We shall give the token the right set of read permissions.
GITHUB_TOKEN for runs on fork do have read permissions, so they likely be able to read:org.

@aaronsteers
Copy link
Collaborator Author

aaronsteers commented Apr 19, 2024

I think it'd be cleaner to discard the use of ${{ secrets.OCTAVIA_4_ROOT_ACCESS}} and replace it with ${{ GITHUB_TOKEN}} We shall give the token the right set of read permissions. GITHUB_TOKEN for runs on fork do have read permissions, so they likely be able to read:org.

Circling back after a while...

@alafanechere - docs seems to suggest that GITHUB_TOKEN won't be enough.

https://github.com/tspascoal/get-user-teams-membership?tab=readme-ov-file#requirements

In order to use this action you need to use a [personal access token] with read:org scope (so the builtin in GITHUB_TOKEN is not enough)

Can we accept this PR as a step in the right direction, and continue to iterate from here? My understanding is that my proposed update is actually in line with the intended logic of the condition that is being replaced.

Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to me

@aaronsteers
Copy link
Collaborator Author

@alafanechere - This is currently blocked on your review. Wdyt of continuing with this increment and then we can continue to tune in future iterations?

@aaronsteers aaronsteers merged commit 3da2fd3 into master Apr 23, 2024
29 checks passed
@aaronsteers aaronsteers deleted the aj/skip-group-membership-checks branch April 23, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants