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

WIP app vs Draft Pull Requests #1506

Closed
petertseng opened this issue Apr 9, 2019 · 5 comments
Closed

WIP app vs Draft Pull Requests #1506

petertseng opened this issue Apr 9, 2019 · 5 comments

Comments

@petertseng
Copy link
Member

In this issue, I weigh the pros and cons of the wip GitHub app versus Draft PRs.
The aim is to consider whether we should leave the wip app installed.
I invite others to contribute other pros and cons.
I give an opinionated recommendation of what should happen for both this repo and the individual track repos, and also invite others to do the same.

Background

In June of 2018, in exercism/discussions#217, the wip GitHub app was installed on all Exercism repos.
My interpretation of the purpose is: Allow contributors to prevent a maintainer from merging a PR that is intended not to be merged.
Contributors might use various signaling mechanisms, but the wip app is an enforcement of one particular signaling mechanism, since there is no other enforcement in place.

In February of 2019, GitHub introduced Draft Pull Requests in https://github.blog/2019-02-14-introducing-draft-pull-requests/.
This has components of both signaling mechanism (there is clear visual distinction of draft PRs) and enforcement (they cannot be merged until marked ready for review).

The two things seem to meet similar needs, so let's compare.

Comparison

What can I do with one that I can't do with the other?

  • Win(?) for Draft PR: I think GitHub could be a bit better about showing the status of multiple status checks. At any rate, having both WIP and Travis CI as status checks has confused the signal before.
    • I point to triangle: Add clarity to test descriptions #1483 . For some period of time, Travis CI wasn't even running, yet (as I recall; the evidence is lost now) there was a green check mark on the PR (because WIP had passed!).
    • I think there are other instances where Travis CI hasn't gotten a chance to register itself as providing status on a PR yet; in such instances the PR also gets a check mark before it has passed Travis CI.
  • Win for wip: I can toggle back and forth between states by adding commits, amending commit messages, or editing PR title as appropriate. With a Draft PR, once you mark it ready for review, I found no way to go back.
    • Use case for toggle: I submit a PR that is ready for review. I get some good feedback. I submit some WIP work on that feedback.
  • About equal: It may not be obvious to contributors that either signaling mechanism exists, so it may be necessary to educate. Use https://help.github.com/en/articles/about-pull-requests#draft-pull-requests or https://github.com/apps/wip can be used to educate.
  • About equal: Possible for maintainers to override the enforcement: A maintainer can also mark a Draft PR as ready for review; depending on configuration, the wip app's status check may not have been marked as required.

Opinionated recommendation

The two alternatives seem about equal in my current evaluation.
I give slightly higher weight for increasing the signal value of the check mark vs X indicator, since it is useful at a glance.
I give slightly lower weight to being able to toggle between states since this repo sees WIP work rarely and toggling even more rarely.

So at preference strength 2/10 I would think it's better to remove wip app from this repo, and use Draft Pull Requests in the appropriate situations.

Regarding individual track repos, I think track maintainers should be empowered to make their own decisions, so it does not make sense for me to make any recommendations applicable to individual track repos, beyond this one:
Track maintainers should consider the pros and cons of the two alternatives and come to a conclusion that makes sense for their individual track.

@cmccandless
Copy link
Contributor

Track maintainers should consider the pros and cons of the two alternatives and come to a conclusion that makes sense for their individual track.

👍

@SaschaMann
Copy link
Contributor

One other downside of the Draft PRs is that they don't trigger CI runs, wip doesn't interfere with them.


I don't really mind wip nor do I really make use of it, so I'm impartial to removing it.

@petertseng
Copy link
Member Author

petertseng commented Apr 17, 2019

One other downside of the Draft PRs is that they don't trigger CI runs, wip doesn't interfere with them.

I'm really grateful you have pointed this out. This is an important downside.


If I read the situation correctly, I think there is not sufficient impetus to move forward with this recommendation on this repo. In that case, let us close this for now. If a track independently chooses to uninstall the app and wants to report on the experience, I encourage the report be made here.

@Stargator
Copy link
Contributor

@petertseng Thanks for bringing this up. I forgot about Draft PRs. I'll try it out and see what I think.

TL/DR: I think there could be some benefits, but I'm not arguing to keep this issue open.

I will say not trigger CI runs, could be beneficial. I know not having the "immediate" feedback is a negative, but on the plus side, it provides a way to not run a potentially unnecessary CI process into a pipeline that could hold up other projects.

Cause Exercism's Travis CI shares a build pipeline, so when X number of repos make CI requests, some may have to wait until an opening is made. If we can limit the number of CI builds to just those that are required (up the to repo), then I can see the value in that.

@iHiD
Copy link
Member

iHiD commented Sep 14, 2019

I remove WIP today without knowing this issue existed. It's been removed from all other projects I work on and I considered Draft PRs to superceded it as a native implementation, so did not consider asking maintainers before doing it. If I'd seen this first I would have checked before doing so - sorry.

It seems the Draft PRs do now have CI so I don't feel there is a good reason to have WIP installed and have two systems in place. I also really like to have the minimal org-wide apps installed as possible. However, if people really want it back, I don't mind reinstalling it.

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

No branches or pull requests

5 participants