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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subsequent passed checks do not update PR status #5

Closed
krishna-acondy opened this issue Apr 2, 2020 · 4 comments
Closed

Subsequent passed checks do not update PR status #5

krishna-acondy opened this issue Apr 2, 2020 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@krishna-acondy
Copy link

Hello @MorrisonCole,

First off, many thanks for creating and publishing this Action. We've been using it on our private repos and find it super useful! 馃檶

I did find a small issue though, and it has to do with PR statuses.
When my PR lint check fails and I update the title of my PR, a subsequent passing check doesn't set the PR status to 'All checks passed'. You can also see this in the screenshot below from your example PR.

Normally, we don't merge a PR unless all checks have passed, and this is showing up some 'false failures'(when subsequent checks have passed).

I did have a look at the source code for the action, and it seems to me that maybe explicitly setting the status to passed when the title matches the regex might help? @github/core doesn't seem to provide a method to do this though.

What are your thoughts?

image

@MorrisonCole MorrisonCole self-assigned this Apr 2, 2020
@MorrisonCole MorrisonCole added the bug Something isn't working label Apr 2, 2020
@MorrisonCole
Copy link
Owner

MorrisonCole commented Apr 2, 2020

Hey @krishna-acondy,

I'll take a look into this and get back to you - thanks for raising it! 馃


Edit: hitting the GitHub API for statuses on a test PR returned:

"state": "pending",
"statuses": [],
"total_count": 0

So it seems neither successful nor failing actions contribute to the ref status in my implementation 馃憖

There is the Create Status API which allows us to set a status explicitly, but ideally we could do this through the action toolkit.

@MorrisonCole
Copy link
Owner

Finally found what was happening here, and it seems to be a common issue actually (see this thread). Seems like GitHub doesn't combine the events because they are from different triggers.

The best workaround for now I think is to have the bot 'request changes' for non-matching PR titles. Ideally, both options would be viable but using the status check API has its downsides at the moment.

For now I'll make this change and release a new version!

@MorrisonCole
Copy link
Owner

MorrisonCole commented Apr 3, 2020

@krishna-acondy released https://github.com/MorrisonCole/pr-lint-action/releases/tag/v1.1.0 which solves this issue 馃帀

You can see it in action here: #33

Let me know if you're happy with the solution - if so, I'll close this 馃槂

@krishna-acondy
Copy link
Author

Great job @MorrisonCole! 馃

This solution works for the moment. We can maybe revisit if anything changes in the way Github handles this. Happy to close this issue. 馃槃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants