6

Pull requests are a great feature of modern vcs hosting. They provide a consolidated view of everything that's happened within a branch's development, including a master diff of the composite change, and all the discussion that's taken place.

My question is this: Is there any downside to issuing a pull request immediately from a new branch?

I suppose traditionally pull requests are issued when someone feels their contribution is ready to be reviewed. Issuing it early can cause some confusion for those accustomed to the more traditional workflow.

On the other hand, issuing early means reviewers can check in and see if someone's changes are going in the wrong direction before unnecessary effort is expended.

Perhaps there are other pros and cons that I haven't considered?

Improve this question
3
  • 7
    On the other hand, issuing early means reviewers can check in and see if someone's changes are going in the wrong direction before unnecessary effort is expended. It should be the responsibility of the person doing development to seek guidance and make sure he's not expending unnecessary effort. After all, they're the ones writing the code, not the reviewer. There's very little incentive for the reviewer to proactively monitor all his pull requests. – Doval Oct 13 '16 at 14:56
  • Ideally that would be the case. However, you might want a reviewer to look over someone's shoulder in certain cases, e.g., new hires or under-performers. Sadly, I've found myself needing to do this on occasion. Even if they can be trusted, is there any real harm or benefit from opening a pull request early? – Neal Kruis Oct 13 '16 at 15:19
  • 1
    Pair programming is probably a better solution if you can't trust a new hire or an under-performer to write code alone. You'll get much quicker feedback that way. There's no harm in opening a pull request early to ask for guidance (but the pull request should explicitly state that). But I can't see using pull requests as a substitute for mentoring being effective. – Doval Oct 13 '16 at 15:41
3

On the other hand, issuing early means reviewers can check in and see if someone's changes are going in the wrong direction before unnecessary effort is expended.

Always assume everyone else is busy. They won't casually check unready pull requests - they have their own tasks to attend to, and trying to figure out the direction from unfinished code, which may contain lots of clutter(things you intend to complete, things you intend to remove etc.), is a time-consuming task. This will also make things harder for you - you'll have to make sure that not just your code is presentable even before it's ready-for-review.

Instead, if you want to inform others of the direction you are going, consider using the bug tracker. These modern VCS hosting services usually have a bug tracker, or are part of a package that has a bug tracker. Bug trackers are not just for bugs - you can also open tickets for features, improvements and refactors. In the ticket you can write in words what you intend to do, and use the mentioning feature that most of them have to notify the people who should care about it. Figuring out the direction from that description is much easier than figuring it from unfinished code...

Improve this answer
2

On the other hand, issuing early means reviewers can check in and see if someone's changes are going in the wrong direction before unnecessary effort is expended.

This should be discussed in design meetings, when features are discussed. Then branches are created, you implement the features and when you think they are ready, create the pull request. Then it is reviewed, you integrate the feedback and the branch is merged.

Basically, a branch with a pull request is usually seen as a set of code finished, but under review (under preparations for merge).

Of course, this is one way of using the tools available. In the end, it is about deciding a policy on what to do and when and, if everybody is happy with it, you can go for it.

The problem with having a pull request opened as soon as the branch is created (or quite soon after the development on that branch has started) is that it creates a lot of clutter, being very hard to know at some point what has been reviewed and what not and what is the progress on everything.

Improve this answer
1

You put yourself in agreement with the potential reviewers. And then yes, there are two purposes for a pull request: To give others a preliminary view of work in progress, and if your colleagues want that you do a pull request, and to offer your hopefully complete and final work for review.

Some people will never want to look at your preliminary work, so you don't create pull requests at that stage.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.