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

Update repo security. #1517

Closed
janiversen opened this issue Apr 25, 2023 · 6 comments
Closed

Update repo security. #1517

janiversen opened this issue Apr 25, 2023 · 6 comments
Assignees

Comments

@janiversen
Copy link
Collaborator

janiversen commented Apr 25, 2023

Something is configured wrong in security, committers (people with write bit) should be able to merge a PR.

Demand is:

  • write bit
  • Branch based on newest dev
  • CI green
@janiversen janiversen self-assigned this Apr 25, 2023
@janiversen
Copy link
Collaborator Author

@alexrudd2 can you please check, that you can merge a pull request now (I removed the demand for approval).

@janiversen
Copy link
Collaborator Author

btw. if you are co-authoring a PR, you cannot approve it, that is standard GitHub security.

@alexrudd2
Copy link
Collaborator

I don't have access to the Branch protection rules (which is fine), but I can guess as to the settings. Note: the screenshots below are from another repo and don't show current values!

write bit

Enabled already.

Branch based on newest dev

I don't understand how, but I think this one works already? Just to be explicit, you always use Squash and merge, right? (The alternative is rebase and merge)
Screenshot 2023-04-25 at 9 48 24 AM

CI green

Doesn't appear to be enabled
Screenshot 2023-04-25 at 9 49 04 AM

Review approval

I believe you were previously able to bypass the requirement of approval because you're an administrator.
Screenshot 2023-04-25 at 9 44 14 AM

Anyways, it's disabled now. #1520 worked for me.

@janiversen
Copy link
Collaborator Author

Please always use squash and merge. Your write bit allows you to merge PR, but not maintain the repo (settings).

I had to disable the CI green (or at least I think so) to ensure you can merge. However that is not a problem if we both are a bit careful.

@alexrudd2
Copy link
Collaborator

I had to disable the CI green (or at least I think so) to ensure you can merge. However that is not a problem if we both are a bit careful.

Hmm, I don't think this was necessary. CI green should be a different status check than requiring review.

I think the below settings are what you want (although you'll have to change test-python)
Screenshot 2023-04-25 at 10 47 13 AM

@janiversen
Copy link
Collaborator Author

Got it to work....you need a status check on top of "require branches...". It works now.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants