RFR: 2065: Update PR labels when new files are touched [v6]
Erik Joelsson
erikj at openjdk.org
Mon Sep 15 17:32:56 UTC 2025
On Thu, 11 Sep 2025 22:59:20 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> This patch is trying to make the pr bot be able to update PR labels when new files are touched.
>> The main idea is from Erik. For a new PR, the bot will run a labelerWorkItem to auto label the PR first, and the commit hash will be stored in a comment.
>>
>> With this patch, here are some new behaviors of the bot:
>> (1) The user didn't issue manual label command before auto labeling, LabelerWorkItem will do the initial auto labeling and store the commit hash in the comment.
>> (2) If the user issued manual label command before auto labeling, the initial auto labeling will be skipped, the bot would still post a comment and store the hash in the comment.
>> (3) The user pushes a new commit or few commits to a pr that already auto labeled, the bot will evaluate the diff between stored hash and current head, then add new labels or upgrading labels to group labels, in the end, update the stored hash in the comment.
>> (4) The user force pushes to the pr that already auto labeled, the bot will evaluate the diff between baseHash and current head.
>> (5) The user issues a command to add a label to the pr(or even the user add the label via the forge UI), the bot will check if the labels can be upgraded to group labels.
>>
>> The side effect of this feature I can imagine is that a user thinks a file is not related to a component and removed it manually, but later, every time when he touches the file, the label will be added back.
>
> Zhao Song has updated the pull request incrementally with four additional commits since the last revision:
>
> - update
> - update
> - review comment
> - fix an issue
bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelCommand.java line 153:
> 151: }
> 152:
> 153: private void addLabels(List<String> labelsToAdd, Set<String> currentLabels, PullRequest pr, PrintWriter reply, PullRequestBot bot) {
If this method is updating the argument set of `currentLabels`, then this should be documented as it's likely not what a caller would expect. Same goes for `removeLabels`.
If we are maintaining a local copy of the current set of labels, shouldn't we reuse that when calling `upgradeLabelsToGroups` as well?
forge/src/main/java/org/openjdk/skara/forge/LabelConfiguration.java line 36:
> 34: * Returns the set of groups that this label belongs to.
> 35: */
> 36: Set<String> groupLabel(String label);
If it returns multiple items, then the noun in the method should be plural.
Suggestion:
Set<String> groupLabels(String label);
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1735#discussion_r2349657632
PR Review Comment: https://git.openjdk.org/skara/pull/1735#discussion_r2349666296
More information about the skara-dev
mailing list