paritytech / pr-custom-review

GitHub Action for complex pull request approval cases that are not currently supported by the Branch protection feature in GitHub.

MIT License

8 stars 4 forks source link

Discuss rules #67

Closed joao-paulo-parity closed 10 months ago

joao-paulo-parity commented 2 years ago

Rendered

Given the reasoning in https://github.com/paritytech/substrate/pull/10951#issuecomment-1072283536, I plan to use this pull request exclusively for discussing repository rules. It will not be merged.

Once a consensus has been reached, we'll implement what is defined in this PR without having to discuss again for each repository.

The plan for this PR is as follows:

Step 1: I'll collect the requirements with @TriplEight Step 2: We'll invite one or more stakeholders to clarify specific details related to their expertise Step 3: We'll request reviews from all stakeholders

32, #35, #36, #38 can be closed after the discussion is finished

TriplEight commented 2 years ago

@bkchr

For the team leads group, please create an empty team on github and then I will work with @dvdplm to fill this team :)

Here you go: https://github.com/orgs/paritytech/teams/parachainleads

TriplEight commented 2 years ago

to summarize here what we've discussed with Basti:

There are now 3 new teams: @paritytech/locks-review @paritytech/cumulus-locks-review and @paritytech/polkadot-review (yet to be populated).

the rules about :lock: s should ping 2 entire teams: @paritytech/polkadot-review AND one of @paritytech/cumulus-locks-review OR @paritytech/locks-review depending on the repo
the rules about changed runtime files should ping 2 entire teams: @paritytech/polkadot-review AND one of @paritytech/cumulus-locks-review OR @paritytech/locks-review depending on the repo
the rest regular PRs are reviewed by @paritytech/core-devs without pinging them. I assume this can be done by not assigning the team, but passively-aggressively waiting and failing a job until someone suitable reviews a pr. Details may be shown in the job's output.

bkchr commented 2 years ago

Otherwise the rules are looking good and are also approved by @gavofyork

gavofyork commented 2 years ago

Looks reasonable to me.

TriplEight commented 2 years ago

Here are the new teams' names we've agreed upon w Basti

joao-paulo-parity commented 2 years ago

FYI I plan to come back to this ASAP which is probably tomorrow or early next week. I've identified https://github.com/paritytech/pr-custom-review/issues/68, https://github.com/paritytech/pr-custom-review/issues/69, https://github.com/paritytech/pr-custom-review/issues/70 and https://github.com/paritytech/pr-custom-review/issues/71 as tasks to be worked on for us to support the requirements discussed here.

joao-paulo-parity commented 2 years ago

The preliminary PRs for the configuration have been submitted.

The application of those configurations through the GitHub workflow should be coordinated with a simultaneous update of processbot (https://github.com/paritytech/parity-processbot/pull/373) since the current requirements are conflicting with processbot's own review requirements.