add process doc for migrating reviewer permissions to owners files #93
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
approved
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
dhellmann
force-pushed
the
reviewer-permissions-migration
branch
from
31c455e
to
c2170a1
Compare
| ### Risks and Mitigations | ||
|
|
||
| If we miss a repository, the list of approvers for that repository | ||
| will also have reviewer permission so we can still merge patches. |
There was a problem hiding this comment.
and if all else fails, the github org owners can manually push changes
|
In general I'm +0 on this. It's more fine grained control (good), but more overhead (not so good). Since merging code requires If we go this route, I can handle the prow config side when the time comes. I havn't found a config example, but I can see it in this part of the code where it handles |
|
looks good to me! This would give more fine-tuning possibilities if needed, per repo or even more fine-grained if needed. Maybe we could already draft a process for adding reviewers, so that we engage into this change with a picture of the end goal. |
For a process, how about just sign-off from 2 existing approvers? That's already to get the PR merged, so just make that the procedure? It should be a relatively low bar to get added to reviewers. |
|
agreed, 2 approvers signing on the PR to add the reviewer sounds good to me! |
See #94 |
|
I propose that we use lazy consensus rules to approve this by 12 June. That's a couple of days after our next community meeting, which gives us plenty of time to discuss it here as well as synchronously. |
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
|
Today I created PRs for all repositories where the reviewer list would have new members based on the policy described here and the command |
At least one PR is reporting an error because OWNERS mentions someone who is not a member of the org. So I think we're going to have to change the Prow settings, then invite people, then add them to OWNERS. |
| <https://github.com/metal3-io/cluster-api-provider-metal3/graphs/contributors>) | ||
| as the initial set of reviewers for each repo. That will allow us to | ||
| complete the migration and we can expand the list further afterwards. | ||
|
|
There was a problem hiding this comment.
This is a good starting point to get enough reviewers to each repo.
| 1. Define a new process for approving reviewers. | ||
| 2. Change the process for approving approvers. | ||
| 3. Change the permissions for managing the CI infrastructure. | ||
|
|
There was a problem hiding this comment.
I am +1 with these non-goals as long as a process of adding the new reviewers is quick. I am only bit concern how we will give reviewers rights, /approve and /lgtm rights in the future. I totally agree that our growing projects needs some sort of gatekeeping, but how and when should we define the process for approving reviewers and promotion of members to have /lgtm for example?
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 dukov also qualifies, but is not yet a member of the org so will be added near the end of the transition Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Prow was previously configured to use its default behavior of looking at public membership of the metal3-io github org to determine who was an allowed reviewer (who can /lgtm a PR). This change switches to using the OWNERS files in each repository, instead. For more information, see: - metal3-io/metal3-docs#93 - metal3-io/metal3-docs#94
|
/lgtm |
testing the changes described in metal3-io#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
Add reviewers who are not already in the approvers list based on the policy described in metal3-io/metal3-docs#93 Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
No description provided.