Friday, August 24, 2018

Pull Requests: How the Process Can Get Stuck

At my current workplace we have the usual Pull Request process:


  • Open a branch
  • Do work
  • Commit all my work
  • Push to the branch
  • Previous steps...ad nauseum
  • When ready open a PR!
A Pull Request can be either a Review or a Merge.  A Pull Request I will heretofore refer to as the PR.

I do Reviews at times, to assure that the work fits everything we are doing and usually its when trying something new, novel, or dangerous.  Once there is some sign off on the Review, and we try to turn these around in < 48 hours, then it becomes a Merge or we generate a new one as a Merge to get all the updates and changes (if any) in the repo.

Merges are fairly straightforward, get some comments or review on the Code.  Does it fit the style guides, are core files updated properly and cross-tested with other peoples Code so we don't have regression failures in other areas.  Once all that is done, again we turn these around quick, or try to, then the merge happens and we go on our way.

Occasionally we have things that throw lots of wrenches in the process.

Big PRs.  I mean HUGE!  Something that touches codes and libraries all over the place, these tend to be rare or due to some major functional change.  A PR like this can take days to review and go over, sometimes with a lot of back and forge, merge conflicts due to others that have been merged along the way.  They become a huge mess and while rare can rear up like a nightmare that sits in the queue for a long period of time until it is finally ready and gets put in, with a little hesitation on that merge button because you just KNOW that within a day something will break and there will be follow-up.  Though usually that's natural, you just hope the follow-up with be slight.

PRs that get opened due to people doing work that will stretch over time, are waiting for core functionality to be finished in another area, or its a huge group effort.  These are the ones that irk me the most because they should never be opened that early in the first place, at least to me.  When I see them I can feel my breath stop and I just know I need a little walk before I continue, they just irk me to no end.
With a global company where people live and work in different areas, and different schedules, it feels like this is the best way to communicate things.  With all of the other tools at our disposal (Teams, Slack, HipChat, Email [remember that one?]) sometimes people think this is the best communication mechanism.  I may be in the minority and disagree, that you can work on a branch with different people if needed, or that branch may need to wait, but there is no need to open it so early, its not ready there is nothing here more than a sign that pops up saying "review me, there was yet another checkin!".  Diplomatically I mention to people there are other mechanisms, but so far its been like tilting at windmills.  Though I dream some day people will listen.

Vacation Days!  Yes, there are even the PRs where the owner opens in the final hours of a Friday afternoon and submits everything, then goes on a well deserved break for the next week and won't be checking emails.  These sit around, with people wondering if the feedback will be looked at, or if that one Unit Test that failed is going to be resolved.  Not much you can do, but wait for them to get back and after catching up, get back to the PR.

Just like a Vacation Day the author can get swamped with something new, or a higher priority task, so the PR sits calmly in queue with comments and questions, which the author decides to get back to but doesn't really notify anyone.  So it sits waiting.

Most of this can be solved with good communication, but at times its just "the way things are".

Personally I think we can always do better, I have a few areas I need to work on constantly so I am a work in progress myself, but I like the see PRs be done quick, easy, and get out of my queue.

Especially when my manager is monitoring that queue and keeps wondering why there is one there.

Shh...it might be sleeping.

No comments: