Question: how do you make a PR that included WIP in a commit no longer a WIP about app HOT 49 CLOSED

Yeah, it'd be nice if there was a way to disable the check on commit messages. Having to tell developers that they need to rebase their commits (some may not be comfortable with rewriting git history, etc) isn't great (especially for what is otherwise a very simple low overhead tool).

from app.

An alternative would be to remove the check in commit messages entirely, I would not make it configurable in order to keep the WIP bot as simple as it is right now

from app.

I can understand making a PR a work in progress when a commit message includes "work in progress" or something similar. But the WIP check should only look at the most recent commit message and, if it doesn't include the phrase, allow the PR to be merged.

from app.

Another option is to add a label “work in progress” instead, then the label will make the app set the pr status to pending. This way you can remove the label anytime you want. Another wip in a commit message would set the label again, etc. would that work for you?

from app.

@gr2m

I like the label approach quite a bit although that would require all repos to add that label in order to work properly then, right?

Quite frankly, I suggest to remove the commit message parsing. Maybe GitHub should just be smarter to populate the title when creating PRs from a set of commits: if any commit includes WIP the suggestion for the PR title should probably include WIP as well. For single commit PRs it would already work, which probably addresses the 90% case anyway...

from app.

Did anyone of you work with the new checks feature & actions? I’m thinking that it might be the perfect feature for this. I’d set status to pending and add an action "ready" to the check, if the user clicks the button, the WIP app receives the webhook and sets the status to green. So far the concept :)

Here is some more info on checks & action: https://developer.github.com/changes/2018-05-23-request-actions-on-checks/. I haven’t seen that in action yet (see what I did there?), did anyone of you?

from app.

Having to tell developers that they need to rebase their commits (some may not be comfortable with rewriting git history, etc) isn't great (especially for what is otherwise a very simple low overhead tool).

You don’t have to, you can use GitHub’s "squash & merge" button. The WIP App setting the status to "pending" will then just serve as a reminder to not accidentally merge it with the other two options.

Does that work for you?

from app.

I’m not sure what you mean exactly? How would you tell the WIP bot to set the state to success if there are still WIP commits on the PR?

I guess I’m saying don’t use commits to indicate WIP ever and instead using titles/labels. That way you won't have to worry about trying to unset the status via commit later.

from app.

The idea with checks was great and I might still implement it, but the problem is the same as with labels, the author of the pull request won’t be able to override the status themselves because custom actions for check runs require write access to the repository.

I’m thinking of implementing a custom override by adding this string to the pull request description:

@wip ready for review

What do y’all think?

from app.

A probot app should be able to manage labels to create the WIP label if it doesn’t exist I believe, although that might require an increase in permissions for the app.

from app.

Why does anybody care if, at some time in the past (some previous commit), something was "work in progress"? How does that somehow indicate that that work is still "in progress" now?

from app.

from app.

@gr2m any updates related to this subject?
Also have you decided if you're going to remove the commit check message or at least only check for the most recent commit?

from app.

It should remove it automatically within a few seconds of the title changing; it could be that GitHub webhooks(?) is being slow to push changes, so the WIP bot doesn't know to action it yet.

from app.

@steveoh Thanks! I see an network error in the logs

Sep 12 13:51:17 event request to https://api.github.com/repos/agrc/gis.utah.gov/commits/f1dcf4b7e4175bcd4f8cb81a0ccff359939b15d9/status failed, reason: getaddrinfo ENOTFOUND api.github.com api.github.com:443

I see plenty of these lately, I’ll check in with GitHub’s API team and probably implement a retry

from app.

Thanks for opening the issue, that’s a very good question, I think we should address it in the README and the app’s description.

You’ll have to update the commits for the pull request. You usually do that locally on your machine and then git push -f to the branch of the pull request. There are some tutorials out there, my fav is https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github, the final chapters are about cleaning up the commit history of a pull request

from app.

from app.

from app.

@gr2m squash and merge is subject to the same branch protection configs as the other types of merges isn't it? So developers would still be blocked if we've configured it that way?

I would be 👍 to removing the check from commit messages, but I know that was originally requested by other people. Maybe worth a poll or similar to see how people feel about it?

from app.

from app.

squash and merge is subject to the same branch protection configs as the other types of merges isn't it? So developers would still be blocked if we've configured it that way?

ah yes of course, sorry I am mostly an admin on the repos I maintain so I can get around that.

Can you just not use it organizationally and instead rely on the labels and
issue titles? What's the harm in keeping it in the tool?

I’m not sure what you mean exactly? How would you tell the WIP bot to set the state to success if there are still WIP commits on the PR?

from app.

yeah I’m thinking of doing that.

from app.

Typically a WIP commit should always be the last commit (e.g. synchronizing between machines), and later be --amended. Having WIP commits merged to master seems wrong to me. Commits should be atomic, i.e. any commit should work on its own.

from app.

I have a commit in this pull that states that it is a work in progress design (commit is atomic though). Not so great that this makes the thing not mergable (without admin rights)

I'm either for disabling that check OR when you add a wip-ignore label (or similar) to the pull it will make it mergable.

from app.

That sounds like a much better way to me!

from app.

I use WIP in my own branches when I need to leave one computer and I intend to finish the work from another machine. This lets me get my work into the main repo and then pull it down into the other machine to continue working. Later, when I'm done and I create a PR, that commit is part of that branch. Eventually, when the PR is accepted, I'll typically squash and merge, so that commit will disappear, but in my case at least the issue with the bot is its use during the PR review step (before squash has occurred). So, my vote would be to either don't look at commit names for WIP, or if you must, only look at the most recent one, not all of them.

from app.

I like the commit message parsing part though. If I meet the multi-desktop scenario, I would squash the commits before reviewing the pull request, because I'm not reviewing the total change, but every single commit. It would be sad if the commit message parsing is removed, without even a chance to opt-in.

Logically, the WIP should not be in the final history at all. It is a status of the pull request, not a description of the commit itself. Commit message should just focus on what is done in this commit.

from app.

Someone pedantic like me would care. Because that "work in progress" has nothing to do with the change itself, it's unnecessary information in the commit message.

But I understand that most user just don't care about history, given all the comments above.

from app.

I usually squash the commits if I want someone else to merge the PR. But it would be nice to be able to do that directly from github.com, without the need switch to the terminal. That’s a feature that would be nice beyond WIP, so if anyone wants to build an app for that, let me know, happy to help make it happen ;) If you are interested, create an issue at https://github.com/probot/ideas/issues and mention me

from app.

from app.

Current branch/PR

from app.

An alternative would be to remove the check in commit messages entirely

Throwing my vote behind this one as well... We just got blocked on merging a PR (which consisted of a lot of individual commits) because somewhere in those WIP was mentioned.

from app.

from app.

I haven’t seen that in action yet (see what I did there?), did anyone of you?

It's used by a handful of services - (travis, appcenter). I believe the intention is to move discreet services that perform checks on code (potentially like WIP) to use the 'Checks' functionality.

from app.

@ctolkien I know some that use the checks integration. I’m looking specifically for the "actions" feature. I will give it a try and let y’all know how it goes.

Another idea to "override" a pending status set by WIP could be to look for a term like "ready" in the pull request title. The benefit over labels would be that the author will be able to set it, even if they are not a contributor to the repository

from app.

you’ll be able to configure terms/locations, see #96. Follow updates at #89. I’m also looking into using checks to manually override a pending status

from app.

#96 looks like a great idea! Best of luck with it 👍

from app.

I notice that even after updating a PR's title to remove WIP etc, that the status does not change. I don't think the bot is notified when the title is changed. It is only checked when commits happen, is that correct?

from app.

I'm seeing one take longer than 31 minutes. Am I being too impatient?

from app.

I just updated this already-WIP PR of mine to not be WIP and back again, and the GitHub statuses on the PR updated as expected: martincostello/api#88

from app.

I don't think the bot is notified when the title is changed

It is. Do you have "WIP" in one of the labels or commit messages?

from app.

only in the PR title. not a label or commit message.

from app.

@steveoh can you share the URL to the PR? Even if it’s private, it will help me to find it in the logs. You can also send me a twitter dm if you prefer not to share it publically: https://twitter.com/gr2m

from app.

agrc/gis.utah.gov#930

from app.

Pending status override is here 🎉

I’ve pushed it to the beta version of the WIP app and would love all your help testing it, please see my update in the 🤖📯 Updates issue.

Thank you all for your patience and help 🙏

from app.

🎉 This issue has been resolved in version 3.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

from app.

you can now add @wip ready to review to override a "pending" status, this requires the Pro plan: https://github.com/marketplace/wip.

All revenue from the "pro" plan will be donated to Rails Girls Summer of Code. I only added the paid plan to make the WIP a real-life GitHub App example. If you cannot pay but depend on the pro features you can add your account with an explanation to the pro-plan-for-free.js file.

from app.

@gr2m we're trying to use @wip ready for review but it doesn't seem to be doing anything - any pointers / debugging? We're on the enterprise plan afaik.

from app.

@mwarkentin Did you put the @wip ready for review into the pull request body or in a comment? It has to be in the pull request body right now

Can you share the PR URL, even if the repo is public? You can send me a twitter dm if you prefer: https://twitter.com/gr2m. I can check the logs

from app.