Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[Meta/PM] Reactivate frame-coders team #13791

Closed
3 tasks done
ggwpez opened this issue Mar 31, 2023 · 11 comments · Fixed by #14122
Closed
3 tasks done

[Meta/PM] Reactivate frame-coders team #13791

ggwpez opened this issue Mar 31, 2023 · 11 comments · Fixed by #14122
Assignees
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.

Comments

@ggwpez
Copy link
Member

ggwpez commented Mar 31, 2023

Reactivate the @paritytech/frame-coders team.

Tasks

Preview Give feedback
  1. 3 of 3
    juangirini
  2. A0-please_review B0-silent C1-low
    juangirini
@ggwpez ggwpez added the J1-meta A specific issue for grouping tasks or bugs of a specific category. label Mar 31, 2023
@bkchr
Copy link
Member

bkchr commented Mar 31, 2023

A team can host in maximum 10 members, after that each member costs $3.09 per month 🥲

People can just add themselves to the relevant files they want to be CODEOWNERS of, not sure the team is needed.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 31, 2023

A team can host in maximum 10 members, after that each member costs $3.09 per month smiling_face_with_tear

🤯

Maybe we can use the custom GHA instead together with codeowners to get something like the team approvals.

@bkchr
Copy link
Member

bkchr commented Mar 31, 2023

with codeowners to get something like the team approvals.

I'm against requiring a fixed set of people to approve certain changes. I mean we already see the bottleneck in Polkadot with the locks review. Or you just want to ping them?

@ggwpez
Copy link
Member Author

ggwpez commented Apr 1, 2023

I'm against requiring a fixed set of people to approve certain changes. I mean we already see the bottleneck in Polkadot with the locks review. Or you just want to ping them?

Hm okay. I was under the impression that the sdk-node team approvals work quite well in Substrate. The FRAME team now consists of ~13 people, so there is not such a big bottleneck i think.
WDYT @kianenigma ?

@bkchr
Copy link
Member

bkchr commented Apr 1, 2023

I was under the impression that the sdk-node team approvals work quite well in Substrate.

There are no required approvals/CODEOWNERS from the team.

We can also create the team and add the people. I think parity should be able to pay these 3$ :D

@kianenigma
Copy link
Contributor

I'm against requiring a fixed set of people to approve certain changes. I mean we already see the bottleneck in Polkadot with the locks review. Or you just want to ping them?

Hm okay. I was under the impression that the sdk-node team approvals work quite well in Substrate. The FRAME team now consists of ~13 people, so there is not such a big bottleneck i think. WDYT @kianenigma ?

If you look at the FRAME PRs, I think often the reviews come from the the actual 13 mentioned developers, but I think especially given the overlap with the common goods parachain, we might stall their work if we mandate @paritytech/frame-coders to approve their PRs.

If so, we can make one approval from @paritytech/frame-coders mandatory, but I would certainly say not two, at least not right now. Perhaps in 6-12 months, in the new mono-repo, we can revisit such a rule.

@kianenigma kianenigma changed the title [Meta] Reactivate frame-coders team [Management] Reactivate frame-coders team Apr 6, 2023
@kianenigma kianenigma changed the title [Management] Reactivate frame-coders team [Meta/PM] Reactivate frame-coders team Apr 6, 2023
@juangirini juangirini self-assigned this Apr 21, 2023
@juangirini juangirini moved this from Backlog to In progress in Runtime / FRAME May 3, 2023
@juangirini
Copy link
Contributor

If so, we can make one approval from https://github.com/orgs/paritytech/teams/frame-coders mandatory, but I would certainly say not two, at least not right now. Perhaps in 6-12 months, in the new mono-repo, we can revisit such a rule.

We can set such a rule but not in a granular way. What GitHub allows us to do is to ask an approval from the code owner(s) of the particular code we are touching, but that rule would apply to the entire repository, we can't specify which folders/files we want that rule to apply to.
So, if we add this rule, any code that has an owner will require its owner to give one approval for any PR touching it.

It looks like this is something we want to do in FRAME, but not sure if the other teams will be happy with this new rule.

@kianenigma @bkchr wdyt?

@bkchr
Copy link
Member

bkchr commented May 9, 2023

https://github.com/paritytech/pr-custom-review should be able to set these granular rules.

@juangirini
Copy link
Contributor

https://github.com/paritytech/pr-custom-review should be able to set these granular rules.

Oh that's great, thanks for pointing it out

@ggwpez
Copy link
Member Author

ggwpez commented May 16, 2023

@juangirini I think @s3krit is in the team only for historical reasons.
Could you please remove him from there? (also means less notifications for him 😆 )

@juangirini
Copy link
Contributor

@juangirini I think @s3krit is in the team only for historical reasons. Could you please remove him from there? (also means less notifications for him 😆 )

happy for him 😄 just removed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants