Add simplistic labeller#2662
Conversation
andrewlock
left a comment
There was a problem hiding this comment.
LGTM 👍 Feel free to ignore all the comments, I got carried away, and this will do the job as-is IMO 😂
| - name: "area:builds" | ||
| allFilesIn: "\\.github\/.*|tracer\/build\/.*|\\.azure-pipelines\/.*|gitlab-ci\\.yml" |
There was a problem hiding this comment.
We should remember to update this label if/when we move Nuke to be top-level
| @@ -0,0 +1,52 @@ | |||
| labels: | |||
There was a problem hiding this comment.
IMO, if this is going to be ingested and converted in Nuke, we should just define them in Nuke, instead of having to parse and interpret from YAML. If you defined them in Nuke directly, you get extra IDE support for the regexes, and don't limit yourself to what can be defined in YAML. I don't think it gains a lot.
There was a problem hiding this comment.
Good point. I didit as a reflex to split conf and code. But I don't mind either way.
| name: GitHubRepositoryName, | ||
| number: PullRequestNumber.Value); | ||
|
|
||
| GitTasks.Git("fetch origin master:master", logOutput: false); |
There was a problem hiding this comment.
I think we should remove this line? Seems unnecessary given you've already done a checkout?
| GitTasks.Git("fetch origin master:master", logOutput: false); | |
| GitTasks.Git("fetch origin master:master", logOutput: false); |
There was a problem hiding this comment.
So my first try was without this line. And it failed fatal: ambiguous argument 'master': unknown revision or path not in the working tree..
This is me fixing the issue after a quick look at StackOverflow :p So I'd happily change to a better solution (or at least adding a comment)
There was a problem hiding this comment.
ah, the next line should probably origin/master. The fetch here is creating a local master branch and associating it with the upstream, which you don't want/need to do 🙂
There was a problem hiding this comment.
Actually, it didn't work. I've roll backed and will merge :) I can revisit later
| allFilesIn: "docs\/.*|.*README.*|.*\\.md" | ||
|
|
||
| - name: "area:installers" | ||
| allFilesIn: "shared\/src\/msi-installer\/.*" |
There was a problem hiding this comment.
Would files here also receive the area:shared-components label below? Ideally I would only want this label and not both
Summary of changes
Adding a simplistic labeler that allows adding label when
[CIApp]orCIApp)profiler/*)Currently, this allows to add
CIApp,Serverless,AppSec,ProfilerandDebuggerlabels easily.In a second commit, I've also added more labels taken from Lucas' PR (#2212) and in a third, modified the release notes to contain Serverless as well
Reason for change
Following #2607, we now categorize release notes based on labels, using the
area-{component}label. So we thought we could revisit having a labeler. I spent a bit of time trying to find one already available but they do not match our needs (matching PR description, matching all files, not removing labels). So I decided to implement a simplistic one as we already have all the tools to do so.Implementation details
Added an action, its configuration and a Nuke target.
Test coverage
Tested on my fork (mainly the first commit)
Next steps
We could imagine also:
type:bugwhen the PR summaryFixessection is filled