pull-request-labeler misbehaving and therefore disabled again

[rgommers]

I cannot find back the previous discussion on this, but this bot is misbehaving again (see screenshot below). I believe the deal was that we'd remove it if it did this again, since it is not okay for it to override maintainer actions. I disabled it manually for now; if someone wants to tweak it once more I'm okay with that, but I'd personally prefer if we just got rid of it.

[agriyakhetarpal]

Happy to take this! It doesn't look like there is a configuration input for disabling the labelling process after a maintainer or someone with the right permissions has removed specific labels on an issue or a PR. Should I remove them or try @j-bowhay's suggestion in #17116 (comment)? There is also a sync-labels option that might be relevant for automatically removing labels based on reverted file changes. Could I get some further direction?

[ilayn]

Hi @agriyakhetarpal thanks for helping us. Currently, it is not providing too much of value for us since labeling is rather the easier part of our work and has no consequence if we forget it. Also it is typically not doing what we hoped that it would do occasionally going rogue as above.

If you know how to tame it and make it less aggressive we would all appreciate it. Especially if reverted changes can be reflected that would be really nice.

If it is a lost cause, I think we are also OK to remove it completely.

[agriyakhetarpal]

I will try a few things on a fork of SciPy and link those items here if I can get to something—otherwise I shall remove the implementation in a PR. Thanks, @ilayn!

[agriyakhetarpal]

Hi, @rgommers and @ilayn: based on the discussion in #17116 and now that the syncing issue actions/labeler#112 has been fixed in the new v4 release of actions/labeler, I would suggest getting back to using that instead of thomasjpfan/labeler@v2.5.1.

1. To mitigate the misbehaviour caused, a good idea could be to run the workflow only when a pull request is opened, or, in rare cases, reopened without having been merged – instead of on every synchronise event.

  pull_request_target:
    types: [opened, reopened]

This would ensure that the bot adds the labels based on the commit at the time of opening a PR and would not run at all after that regardless of whether the sync-labels input is marked as true or as false.

2. I tested the workflow in a pull_request event on my fork (since pull_request_target does not usually run on forks) in Trying to debug pull-request-labeler agriyakhetarpal/scipy#2 and it has worked without any problems. I manually removed an automatically added label following which the workflow did not execute. Some more information is available on the PR thread with steps as to how I proceeded with debugging the workflow(s).

3. Another method of achieving the same thing is to add a label to the pull request and use that to decide¹² whether to execute or skip the job (tested on the above PR in my fork)

${{ !contains( github.event.pull_request.labels.*.name, 'reviewed') }}

and use this condition to check for a particular label, say, "reviewed" in this case, and skip the job if the label is found. One more way of doing the same thing, perhaps cleaner, would be via

  pull_request_target:
    types: [unlabeled]

which would make the labeler run on unlabeled PRs and make it not run upon further activity since the PR would already have been labeled by itself, in some parasitic sort of fashion. However, this would mean that at least one label should remain on the PR. The "reviewed" label is just an example, it could as well be one of these "scipy.*" labels if a new label is not to be added to the repository

4. The last recommendation is to move the permissions currently associated with the workflow level to the job level for security reasons, given that this workflow runs on a pull_request_target event rather than a standard pull_request event and it would always contain just a singleton job.

Thanks for the patience for letting me get back on this and apologies for the delay caused. I have presented multiple potential solutions wherein some of them should provide a failsafe even if pull-request-labeler does not work as expected again (they can be combined too!), please let me know if adding the sync-labels option set to false should be fine or if there is anything specific or more to be desired with the functionality that I can work on and contribute by writing a PR for.

Footnotes

1. https://github.com/orgs/community/discussions/26712 ↩

2. https://stackoverflow.com/questions/59588605/how-to-check-for-a-label-in-a-github-action-condition ↩

[lucascolley]

Hi @agriyakhetarpal! IMO, it would be great if you could open a PR for this with the types: [opened] method (no need for reopened IMO). I don't think there is any need to consider which labels are already on the PR - we just want this to run once when the PR is opened.

The last recommendation is to move the permissions currently associated with the workflow level to the job level for security reasons

Sounds good.

please let me know if adding the sync-labels option set to false should be fine

It looks like the default is false, so just leave it out. I don't think it would make a difference anyway when we only run on [opened] events.

---

Feel free to ping me on the PR once it is open!