How long to wait before merging a PR?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

How long to wait before merging a PR?

David Blevins-2
I got an offline question on how long you should generally wait before merging your PR.  It's a great question and since we are adding new committers, I'm answering here for everyone's benefit.  It's one of those topics that's healthy to reiterate every 5 years or so as people come and go.

Apache projects generally fall into two categories of how they like to operate, both are allowed and deemed "ok"

 - Commit-then-review (CTR).  In this model you just push your changes and others can review them afterwards.  If more changes are needed that can still occur via more commits.  In this mode PRs are simply a courtesy.

 - Review-then-commit (RTC).  In this model you should not push changes directly unless they have been reviewed and approved first.  In this model PRs are mandatory for any change of any size.

TomEE is a commit-then-review project.  The simple answer to the question of "how long do I have to wait to merge my PR" is you don't have to wait.  It is up to you if you want to submit a PR and up to you how long you wait for feedback.

Here's how I like to behave:

 - When: I tend to push directly without a PR for most routine work.  Once in a while I get a chunk of time and use it to completely rewrite something or create a big new feature.  In those situations I like to create PRs to get feedback so people have a chance to object or at least ask questions.  We'll all have to maintain it after all, so I like to get some buy-in.

 - How Long: I don't tend to wait long.  If there's immediate discussion and people are engaged (yay) I'll happily leave it open for as long as people want to discuss.  If no discussion occurs, then I'll just push it.  In the "no feedback" scenario, I'm unlikely to wait even a week; there are only 52 of those in a year, so I don't like to waste them.  I like to give it a day or two and push if there isn't any feedback.

What kind of feedback I find useful:

 - Useful: As PRs are voluntary, when I file them I'm really looking for a thumbs up or thumbs down on the general concept.  Do people think it's a good direction for us or a bad one?  Looking for that gut reaction kind of feedback.

 - Fine, but not the goal: I'm not really looking for a "can you change this one line" kind of feedback on the PR.  To be clear that is fine to say, completely encouraged and even enjoyable, but can easily be done in the master branch in commit-then-review style.  If I'm happy with the concept, I'm likely to want to merge their PR (or have my PR merged) and do further refining with other PRs or commits.  To me it's a lot more fun to be working with people in a place where both of you can commit as equals as it just feels more collaborative and fun as opposed to one of you making demands and the other being held up meeting them all.

 - Not great: If I'm getting one response every few weeks over the course of months, that's not enough to keep a PR open.  I'd just push.

I like to use PRs once in a while to get feedback as I know some of my changes are kind of big and hard to digest and once I push them in the odds of getting feedback drop significantly.  Because they're big and hard to digest it also means I'm unlikely to get any feedback, so I just do my best.  As you do bigger and bigger changes you'll find the same thing occurring and you'll also have to learn to make do.

So there's my $0.02 :)

Everyone has a slightly different style.  The project is commit-then-review so you are welcome to experiment with all aspects of PRs and find your own style.  You have that freedom.


--
David Blevins
http://twitter.com/dblevins
http://www.tomitribe.com


smime.p7s (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: How long to wait before merging a PR?

David Blevins-2
Important note.  If you do submit a PR, you should still come to the list and propose/discuss your idea.

I know people see that as duplication, but this project is 20 years old and has used CVS, SVN and now Git on GitHub.  Who knows we may switch to Gitlabs or some other thing yet-created.  For the website we used Confluence, Apache CMS and now a custom git thing.  Things like JIRA come and go as well and get replaced by git issues.  We moved from IRC to Slack.  The wheel keeps moving.

The mailing list and its archives have outlived all of that.  You're strongly advised to use it.


--
David Blevins
http://twitter.com/dblevins
http://www.tomitribe.com


smime.p7s (1K) Download Attachment