thehanimo / pr-title-checker

An action to automatically check if pull request titles conform to Contribution Guidelines
MIT License
106 stars 36 forks source link

ignoreLabels is removing labels #40

Closed bcchenbc closed 1 year ago

bcchenbc commented 1 year ago

I'm experiencing a strange behavior from the bot, as shown in the screenshot below. image

My config looks like below (emoji is removed for clarity), the goal is to have the bot to check if the title is prefixed with a ticket number, add a No Ticket label if not. But do not perform the check if the No Ticket label is already present. It looks like the line 10052 https://github.com/bcchenbc/pr-title-checker/blob/main/dist/index.js#L10052 is removing label, which I guess is not intended. Would you mind taking a look? Thank you!

  "LABEL": {
    "name": "No Ticket",
  ...
  "CHECKS": {
    "ignoreLabels" : ["No Ticket"],

(I created a PR https://github.com/thehanimo/pr-title-checker/pull/39 to remove the line, but it's showing me 8,814 additions, 8,815 deletions not shown because the diff is too large. Please use a local Git client to view these changes., so I'm creating this issue guessing that you might have better way to fix it 😅 )

thehanimo commented 1 year ago

Your changes are in dist/, which is the build output of index.ts. All changes have to be made in the source file, which is index.ts.

For the problem itself, I think the tool works as intended. In your case, I'm assuming the title has a ticket number but you manually want to add a "No Ticket" label to it? I'm not sure if that is the use case here but I'm sure that the tool does not (and possible cannot) support it. It takes away the base logic of "if not formatted, add label; if formatted, remove label".

Does that make sense?

thehanimo commented 1 year ago

If you do want it to work, you could define a new label such as "verified" or "lint-disable", add this to ignoreLabels and tag PRs that you don't want the bot to check with it.

bcchenbc commented 1 year ago

Thank you for prompt reply! & for clarification (re: index.ts, I'll make a change to this line https://github.com/thehanimo/pr-title-checker/blob/main/index.ts#L285 when available, if that works for you)!

The PR I created did not have a ticket number, and I didn't modify the title during the process. I was hoping that the bot would add the No Ticket label, and keep it there. Would you mind checking if my experience was expected? What happened was something like:

  1. (the 10 mins ago line) I created the PR (the bot added No Ticket as expected).
  2. (the 6 mins line from me) I added other two labels,
  3. (the 6 mins line from bot) the bot got triggered again, but removed the No Ticket label (which is unexpected).
  4. (the 5 mins line from me) I tried to added back the No Ticket label, but
  5. (the 5th line from bot) the bot got triggered again and removed the label.

I guess I'm a bit confused about how to interpret ignoreLabels. I was expecting that if the bot sees a ignoring label, it'll do nothing. If the title is not formatted, it'll add the label at the first execution (when PR was created). But it'd do nothing in following executions (for example, when modifying other labels), since the ignoring label is there. It'll be really nice if I can still with one label, so... really hope that it could work 😄

thehanimo commented 1 year ago

You are almost correct. ignoreLabels does mean it'll do nothing, but also reverts previous actions, i.e., remove tag if it was previously added.

What is unexpected in your flow (likely a bug somewhere) is step 3. If the title was the same in step 1 and 3, the bot should behave in the same way by adding the No Ticket tag. Are you sure there was no change in the title?

thehanimo commented 1 year ago

Oh right. It does work as intended. It "undid" its previous action of adding the tag (which in your case coincides with the one to be ignored)

bcchenbc commented 1 year ago

Ah, reverts previous actions tripped me 😅 Thank you for clarification!

(Just confirmed, I did not change the title, btw.)

bcchenbc commented 1 year ago

Removing ignoreLabels (setting it to []) works just fine for me. Thank you!