search

thoth-station / template-project

This is a Template for any Python based project
GNU General Public License v3.0
7 stars 9 forks source link

Add all the Thoth team members to the list of approvers #24

Closed codificat closed 3 years ago

codificat commented 3 years ago

Related Issues and Dependencies

Suggested after thoth-station/prescriptions#18517

There are other PRs updating existing repos.

This introduces a breaking change

Description

This is to add all the current members of the Thoth team as approvers, so that future projects inherit that list and everyone in the team is in a position to /approve changes to the repos.

I also added myself as a default reviewer. I am not sure if we should change the list of reviewers, or even have a default one. But I'm happy to receive requests to review new PRs (that I can redirect :smiley:)

Side note: sorting the list alphabetically.

sesheta commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign goern after the PR has been reviewed. You can assign the PR to them by writing /assign @goern in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/thoth-station/template-project/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
codificat commented 3 years ago

please add all to reviewers as well, if we are going for approvers.

Done, added everyone to both approvers and reviewers.

codificat commented 3 years ago

Done, added everyone to both approvers and reviewers.

On second thought, I'm not sure if we want everyone as a reviewer by default... different repos have different experts who are best suited for review, and we should probably fine tune this, possibly relying on SIGs as specified within the OWNERS_ALIASES file? as per the one in the core repo.

Should we leave the list of reviewers in this repo as the people who should get pinged about contributions to the template itself?

codificat commented 3 years ago

Done, added everyone to both approvers and reviewers.

On second thought, I'm not sure if we want everyone as a reviewer by default... different repos have different experts who are best suited for review, and we should probably fine tune this, possibly relying on SIGs as specified within the OWNERS_ALIASES file? as per the one in the core repo.

Should we leave the list of reviewers in this repo as the people who should get pinged about contributions to the template itself?

Let's agree on this first and /hold meanwhile. If we limit the list of reviewers in this repo, let's also confirm who they should be. Maybe leave the current list of reviewers instead?

goern commented 3 years ago

I think the default should be a limited number of approvers, there are the members willing to take responsibility to merge changes into the code base.

reviewers should be the ones, that have knowledge of the code base, and can tell if a code change is good or needs improvement.

codificat commented 3 years ago

Understood, and I agree in principle.

But at the same time there's also a need to make some things more agile. A current example is AICoE/sefkhet-abwy#176 where @fridex is in thoth's maintainers list but not an approver for the repo.

There's an related agenda item in this week's the community meeting, will follow up there.

codificat commented 3 years ago

We discussed this in the community meeting and the conclusion was that the default (and template) list of approvers/maintainers/reviewers should be a minimal set.

Additions are (very!) welcome to each repository, based on everyone's interests and expertise.

/close this one, but encourage anyone to send PRs to specific repos to add themselves

sesheta commented 3 years ago

@codificat: Closed this PR.

In response to [this](https://github.com/thoth-station/template-project/pull/24#issuecomment-954671514): >We discussed this in the community meeting and the conclusion was that the default (and template) list of approvers/maintainers/reviewers should be a minimal set. > >Additions are (very!) welcome to each repository, based on everyone's interests and expertise. > >/close >this one, but encourage anyone to send PRs to specific repos to add themselves Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.