Rob Williams is a probabilistic Lean coder of Java and Objective-C. Rob is a DZone MVB and is not an employee of DZone and has posted 170 posts at DZone. You can read more from them at their website. View Full User Profile

Few Thoughts on Pull Requests After a Week Or Two

05.13.2011
| 3179 views |
  • submit to reddit

Still overall very positive on the new automerging pull requests on GitHub, but there are a few things that I think would benefit them.

First off, I want to embed comments in the code upon creation of the pull request

Back when we were using Crucible, we had a flow where the developer would initiate a review as a way to walk people through what was changed. It‘s easy enough to attach some general comments to the code in pull requests, but spelunking down into the code and attaching comments is a real nuisance. When you read the documentation, it‘s clear that they think that someone else should come in and initiate specific discussion points in the form of questions. This is not really a good model for a number of reasons: it encourages the idea that code be thrown at a wall and if the result is not comprehensible, the team can waste time asking for clarity.

There are also a lot of cases where something is being done and there is probably reason to consider alternatives. For instance, I was using an @Embedded entity the other day but did not want an ordinal nor a long string taking up space repeated on each row. Rather than go into a digression over an ‘optimization‘ (prematurity being the root of evil), I would tend to want to commit that and open that issue up for discussion, or note it. Of course, with Issues 2.0, that would be another option for this particular case. Probably a better example for the discussion stuff would be to connect the dots on things like pattern implementations or code that was done to tie a bunch of things together. The last commit of the day today was setting up an injected property that made it possible to turn off all startup jobs when running locally (so developers‘ instances were not spawning quartz jobs each time a restart was needed). Made the jobs extend a base class and check for the presence of the property and then not schedule if the switch was off. This cut across some varied terrain in the codebase. Painting a picture and explaining what was done would have been nice.

For some reason, a lot of my commits are not getting the commit comment added as the pull request summary. Will have to look into that.

It is cool that you can embed images and use markdown in the general notes; will definitely make use of that.

On the whole, it‘s pretty remarkable how much thirst quenching the GitHub offerings accomplish. It‘s a testament to agile that often times a minimalist approach pays the highest dividends. Right now, we are pretty much using just AgileZen and GitHub and it's better than our previous arsenals by a mile (last year we used Rally and before that XPlanner with Crucible and FishEye).

Still, the rhetorical potential is not what it could be. The fact that the example chosen for the docs is inane to the point of lunacy is pretty apropos: the social part of the equation has yet to set.

 

From http://www.jroller.com/robwilliams/entry/few_thoughts_on_pull_requests

Published at DZone with permission of Rob Williams, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)