search

zopefoundation / meta

Meta issues concerning many/all of the zopefoundation repositories.
Other
7 stars 6 forks source link

Automatically set Branch protection rules #191

Open icemac opened 1 year ago

icemac commented 1 year ago

Currently I set the "required status checks to pass before merging" by hand:

Repositories containing C code or testing against Windows or MacOS have other status check names.

The actual list of status check names depends on the configuration via meta/config so it should be set when running it.

Update:

mgedmin commented 1 year ago

Note that branch protection rules interfere with people using zest.releaser to make releases. The failure mode is pretty annoying: the git push is rejected after the release is already pushed to PyPI, and so you end up with a release that doesn't have the corresponding source available on GitHub, unless people notice and take steps to remedy the situation (e.g. by creating a pull request containing the release tag and the post-release version bump commit).

When this has happened in the past I would go into GH repository settings and turn off branch protection.

Do we have a better solution?

mgedmin commented 1 year ago

(I forgot to mention that people with admin rights to the GitHub ZopeFoundation org are exempt from branch protection rules and can use zest.releaser freely, without noticing that there is a problem for other maintainers.)

dataflake commented 1 year ago

I don't use zest.releaser at all. I'd have a problem with turning off branch protection just because one specific tool can't deal with it.

d-maurer commented 1 year ago

I don't use zest.releaser at all. I'd have a problem with turning off branch protection just because one specific tool can't deal with it.

As @mgedmin has pointed out, the problem would go away if the people doing releases were admins.

If this would give them too many right, maybe "Personal Access Tokens" ("PAT") might help. Apparently, they can give fine grained rights - maybe there is a right "ignore branch protection". In this case, releasers potentially could get a PAT with this right to do their release work.

mauritsvanrees commented 1 year ago

I don't use zest.releaser at all. I'd have a problem with turning off branch protection just because one specific tool can't deal with it.

This is not because of the use of zest.releaser. The problem is that a simple git push fails.

icemac commented 1 year ago

This is not because of the use of zest.releaser. The problem is that a simple git push fails.

This failing git push may be not recognised by the person cutting the release. Could there be an ability to create a branch for the release commits so they can be merged via a pull request?

The other question would be: Do we have too many people who are able to cut releases? (Some having PyPI rights have left Zope or even Python years ago. Some care just for a specific package where they should be able to cut releases.) From a security and quality standpoint I'd like to see successful pull request before changes in master. I see protected branches and required successful test runs as a valid tool to keep the quality.

Maybe we need a more clean policy regarding releases. (We could even create a release team and allow them to push to master via Restrict who can push to matching branches.)

What do you think?

icemac commented 1 year ago

Another thought: We are not enabling Require a pull request before merging for master branches so the scenario After-a-release-I-cannot-push-my-changes-to-master should not happen at all.

dataflake commented 1 year ago

There's a setting "Allow specified actors to bypass required pull requests" that can be applied to people or teams. We could set up a new team specifically including the intersection of those who are not admins but who make releases, and then set that team as allowed to bypass PRs. I would bet that's a pretty small group.

dataflake commented 1 year ago

I have created a new team "Release Managers" at https://github.com/orgs/zopefoundation/teams/release-managers/members. Feel free to add members there.

The group needs to be added to each affected repository in the repo's "Settings" | "Collaborators and Teams" as well before it can be selected for "Allow specified actors to bypass required pull requests" when editing a branch protection rule. Adding a team to a repo can also be done at https://github.com/orgs/zopefoundation/teams/release-managers/repositories by searching for and selecting a repository. There doesn't seem to be a way to just force-add a new team everywhere.

icemac commented 1 year ago

There doesn't seem to be a way to just force-add a new team everywhere.

I hope that this can be done via the API, so it could be included when running meta/config scripts.

dataflake commented 1 year ago

There doesn't seem to be a way to just force-add a new team everywhere.

I hope that this can be done via the API, so it could be included when running meta/config scripts.

It can be done with the API, and I just did that. I added the "Release Manager" team to all non-archived repositories within the zopefoundation organization and gave it the maintain role. Here's the script I wrote: https://gist.github.com/dataflake/f33932efa8fa6423ff2907708664f936

The team doesn't necessarily make sense in all of those repositories, but it won't break anything and can be deleted easily.

icemac commented 4 months ago

Status: Allow release managers to push to master without needing a PR is not yet implemented.