Remove labels for reverted changes

[heshamMassoud]

Reproducing:

1. Change something in a file in some PR.
2. A label will be added to the PR that matches the location of the changed file.
3. Revert the changes on the changed file. (i.e. the PR does not contain changes in such file).
4. The label will still be there on the PR. But it should be removed, since the PR does not affect such file anymore.

[damccorm]

As of now, this is expected behavior. Its actually not clear to me that we would want to change this for a couple reasons:

1. Labels might apply even if they don't match the file paths from the yml file. To use a contrived example, I might have a documentation added label that I automatically apply to any changes to the README using the labeler action, but manually apply if there are significant comments added in the code - should the labeler remove that label if further changes are made?
2. For searchability I personally wouldn't want labels taken away. If I know a PR is under a certain label, I'll often use that label to find it again, and it would be confusing to not find the PR because of changes I don't have visibility in to.

[heshamMassoud]

Thanks for the reply @damccorm

Regarding point#1, that is definitely a valid argument. Could be solved by only being able to remove labels which have been added by the same "labeler" bot and not by someone else. But I am not sure if github exposes such info (who added a certain label) 🤔

Regarding point#2, I am not sure I am with you on this one. Not having the label any more on some PR, simply means that the PR doesn't contain any changes categorised under such label (at least not anymore), which in turn, means that searching using a certain label should only result in PRs which affect codebase fulfilling such category, which is correct. If anything, then searching for such PR should be done with another query/filter.

[damccorm]

Yeah, I think I agree with your analysis of my second point, so I guess we're just left with the first one.

The more I think about this, the more I think we want it though. I guess if you really want to do what I'm describing you can create another label documentation added (manual) or something that doesn't appear in labeler.yml. It also just doesn't feel like the main use case. Maybe we flag it under an option or something so that we can turn it off.

Probably won't get to this immediately though, #12 feels a little more pressing to me.

[Malinskiy]

In a multi-module project the labeler might be used in the following scenario:
Assume the setup where we have 2 modules A and B and we assign label A and B depending on the affected paths.

1. Dev creates a PR with changes to modules A and B
2. Reviewer asks the dev to separate the PR into two changes separately (or refactor) and current PR now affects only A
3. Since labels are not removed or updated initial PR is still labeled as A + B which would be confusing

Since there are multiple competing use-cases here I'd like to see an option to choose the behaviour to update the labels created by the action somewhere in the pipeline definition.

[pzavolinsky]

I'd like to see an option to choose the behaviour to update the labels created by the action somewhere in the pipeline definition.

Same here, I was planning on using labeler to add a cr-no-tests label to PRs that have no tests to ease the code review process (and I can think a couple more hints like these that would be super useful in prioritizing CR).

My initial config was this one:

cr-no-tests:
  - '!features/**/*.feature'

For this scenario I clearly want my cr-no-tests label to go away the moment a test appears.

We could get this going with:

• const syncLables = !!core.getInput('sync-labels', {required: false}); here
• else { labelsToRemove.push(label); } here
• if (syncLabels && labelsToRemove.length) { await removeLabels(client, prNumber, labelsToRemove); } here

[kevinbuhmann]

I have opened pull request #63 which implements this issue (thanks for the pointers, @pzavolinsky).