mui-toolpad
mui-toolpad copied to clipboard
mui
[core] Add CI check that the PR has label
This is probably too early to enforce at least one GitHub label on PRs but part of https://www.notion.so/mui-org/Retrospective-W32-33-cfd4332ad6a54f4593b453a8ec0bf91f#edf81bb00b504c8abd5b970aff98a77a. I believe that we eventually want to get there. Enforcing labels would solve two problems:
- Scaling reviews. It's what we use in https://www.notion.so/mui-org/0c8156b0b1634ea5bfd903dd005ec1cf?v=b9e94cc3ea554e668e7c54d585910569
- Guiding high-level resource allocations. https://www.notion.so/mui-org/KPIs-1ce9658b85ce4628a2a2ed2ae74ff69c#06950ec6688840549cfc9429635f4298
I have copied https://github.com/mui/material-ui/pull/32886
Are there any downsides if we try to enforce at least one label? Worse case scenario we can add just any label to make PR pass and document why we could use other meaningful labels, or if we need to create new ones 🤔
Are there any downsides if we try to enforce at least one label?
It's a bit more friction, the GitHub Action takes ~20s to run, a time you have to wait before you can merge your PR if a label is missing.
As a general rule, I would personally be in favor of us feeling pain around the lack of labels before merging this.
It's a bit more friction, the GitHub Action takes ~20s to run, a time you have to wait before you can merge your PR if a label is missing.
Don't we need to wait for like 10-20 minutes before circle CI jobs complete? 🤔
I was also thinking if we could run GH action but make it optional in order to merge PR? That way it would introduce little bit of noise to nudge us to start using labels, but would not block merging of PRs?
I would personally be in favor of us feeling pain around the lack of labels before merging this.
I'm fine with merging, but this resonates with me. Without a clear purpose, I'm not sure we would be investing much time in the quality of our labeling.
Maybe something like a very short reminder in the PR template would work for now? Even though if it only takes 20s for the CI to report missing labels it sounds fine too.
I came back today to this PR after seeing some of the regressions, which made me think that having some metrics could help us identify if we write enough tests or not, identify if we focus enough/too much on bugs. Forcing at least one label could cover:
- https://www.notion.so/mui-org/KPIs-1ce9658b85ce4628a2a2ed2ae74ff69c#5d672eaa191e41108d10ad84f3a3e6cf
- https://www.notion.so/mui-org/KPIs-1ce9658b85ce4628a2a2ed2ae74ff69c#06950ec6688840549cfc9429635f4298
I think we should label in function of what we want to measure. We're currently labelling bugs and regressions.
some metrics could help us identify if we write enough tests or not, identify if we focus enough/too much on bugs.
If we want to effectively measure these two things, which extra labelling would be required?
in function of what we want to measure.
@Janpot At our scale, a few ideas of concrete problems that it could solve:
- To find older work. We are getting close to
#1000. We might want to find discussions on older work, without the labels to filter out issues/PRs, good luck. - To provide clarity on what the issue/PR is about. Sometimes simply choosing between, a bug, a new feature, support: question, and docs, can require a very deep understanding of the pain point.
- To delegate the area of ownership, with broad product scope labels, we could start to prepare when we reach the point where the is too much activity on the repository for a single person to be able to see everything.
- (what I mentioned in https://github.com/mui/mui-toolpad/pull/849#issuecomment-1276801189) Maybe to track
regressions, I'm not sure, maybe it's helpful as a broad feedback loop for us to: if there are too many then it means that we are moving without testing enough.
To provide clarity on what the issue/PR is about. Sometimes simply choosing between, a bug, a new feature, support: question, and docs, can require a very deep understanding of the pain point.
Yes, those labels I agree with.
To delegate the area of ownership, with broad product scope labels, we could start to prepare when we reach the point where the is too much activity on the repository for a single person to be able to see everything.
I'm not sure I currently see clear lines where we can determine ownership. I was hoping for the future to do that more along the lines of CE, EE, cloud, docs. The current labels like "feature: queries" don't feel very useful to me. e.g. I change something that affects all queries, does it go in "core" or in "feature: queries"?
The current labels like "feature: queries" don't feel very useful to me.
We can remove these, until we see a clear pain point it solves. At this point, my main assumption is that the feature labels can help to find patterns of bugs, find duplicates, but if the volume is low enough to be able to see all the new issues, then yeah, agree, it's overkill at our size.
Ok, sounds good. So in terms of which labels have to be applied, how about
- core
- docs
to define area of ownership (We can add more labels when necessary), optionally accompanied by
- bug
- regression
- feature
- enhancement
- ...?
to define the nature of the PR?
Question: With this github action, can we define which labels to pick from? e.g. PR must have one blue label (core/docs) and one yellow label (bug/regression/..)
Question: With this github action, can we define which labels to pick from? e.g. PR must have one blue label (core/docs) and one yellow label (bug/regression/..)
This GitHub Action doesn't support it. @mnajdova wrote it to help us find cases where we forget about applying any labels.
The problem we had was sometimes we forget about adding the labels altogether. It assumes that once we remember the labels, we then do it "correctly". It's not perfect, but honestly, I don't see the need to be more granular.
Does the labels option allow for specifying which labels are expected?
Based on https://github.com/mheap/github-action-required-labels#examples, yes, I think that it could be possible.