Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Label changes in GitHub are ignored after a PR has been opened #496

Open
longwave opened this issue Mar 20, 2017 · 10 comments · May be fixed by #639 or #804
Open

Label changes in GitHub are ignored after a PR has been opened #496

longwave opened this issue Mar 20, 2017 · 10 comments · May be fixed by #639 or #804
Labels

Comments

@longwave
Copy link

longwave commented Mar 20, 2017

The recently added label whitelist/blacklist feature has an annoying bug: if you whitelist a label but forget to add that label when you first open the PR, adding the label later and commenting "test this please" still doesn't trigger the PR builder. The log contains entries like this

Mar 20, 2017 5:46:02 PM org.jenkinsci.plugins.ghprb.GhprbPullRequest checkWhiteListLabels
INFO: Can't find any of whitelist label.

even though the whitelist label is now set on the PR in question. The only fix appears to be to close and reopen the PR with the label in place to begin with.

This was also noted at #449 (comment)

@benpatterson
Copy link
Member

@longwave thanks for logging this. It was a known limitation when we implemented label support; however I see that I did not make that very clear. Pull requests webhooks would need to be configured to include label events, and we'll need to enhance the plugin to ingest them properly.

I can't get to this for awhile but wanted to acknowledge that you're right; it's an issue. I'll be glad to review any PRs in the meantime, and will otherwise get to this when I can.

@senorsmile
Copy link

+1 This just hit me, and it took me a while to find this bug report. It seemed to simply not be working until I realized this.

@jfrederick
Copy link

+1 Would love a fix for this, to support a complete label-centric workflow

@Bamieh
Copy link

Bamieh commented Jan 17, 2018

@benpatterson I am trying to find the source of the issue. It would be awesome if you provide some guidance on where the related code lives.

I am suspecting this place GhprbPullRequest.java#L227.

@benpatterson
Copy link
Member

@Bamieh I can only give some vague advice, i'm afraid. (I no longer maintain this plugin.) However, I'm still convinced it would not be hard to add support for this.

The line you point out...what that does is check labels to see if the pull request has the whitelisted ones. Arguably that part of the logic is "working". The problem is, at this point no labels are seen. Or maybe more accurately, no new labels are seen.

I think one place to start might actually be in GhprbCause.java. You'll see that we don't take any label information when we receive a webhook. What I do not know, is what webhooks look like with label information. Hopefully, it's just an additional attribute, and you can put in a statement or two that initializes label information for the Cause. So, it could be that you want to change your webhook, and then ensure you are pulling out label information for every new webhook.

This may or may not help, but I also draw your attention to a key dependent library, which is the github-api dependency. Specifically, there is a GHLabel class here. You may or may not need that class, but I thought I'd point it out in case it's handy.

This is my best guess at the moment; I could be wrong. But hopefully this at least gives you or someone else a good place to start.

@Rechi Rechi linked a pull request Mar 4, 2018 that will close this issue
@maxdanilov
Copy link

maxdanilov commented Mar 20, 2020

Hi @bjoernhaeuser , is there any movement on this issue?
This PR has been suggested almost two years ago as a solution: #639
Any reason why it's still not merged? It seems like a fundamental drawback in the plugin.

@ideepika
Copy link

ideepika commented Apr 9, 2020

+1 for the feature request

@anikamurarka
Copy link

Hi, is this fixed?

@initialzero
Copy link

I would love to have this fixed! Who can I bribe?

@mattymo mattymo linked a pull request Dec 11, 2020 that will close this issue
@aali0730
Copy link

Hi can we merge the fix for this? #804

Would be extremely useful if we can get this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
10 participants