bgror

Don’t “Push” Your Pull Requests »

Created at: 19.12.2011 20:51, source: igvita.com, tagged: Open Source opensource oss review

You've run into a problem, or you have an itch to scratch, so you fire up the editor and dive right in. On a good occasion the patch is simple, so you pat yourself on the back, fire off a pull request and sit back and watch - even though it is a simple patch, past experience shows that often you will have to incorporate some feedback before it will be merged upstream.

Then, every once in a while, you stumble on that gnarly problem - the one that looked simple on the surface - and you spend the better part of your day on it, or more, and the further you get in, the more committed you become to see it through. Finally, you emerge victorious with a 300+ line patch, a dozen modified files, and a feeling that you have just done the world a favor - time to fire off the pull request!

This is where it all breaks down. You expect gratitude for all the time, work, and effort you've put in, but the maintainer looks in horror at the patch as he tries to enumerate all the reasons why he can't accept it. Half the time, he doesn't even know where to start, so he picks a nitpick and digs in - your contribution is going nowhere fast and neither side is to blame.

Don't "push" the pull request

Contributions are always welcome, but surprise patches are mostly just a burden. Yes, you are offering your help, but someone else will have to maintain your code over the long term - get their buy-in first, avoid surprises. Even worse, a localized change to address a specific problem will often miss the full implications to the project: other existing use cases, future roadmap plans, or overall architectural decisions. A good idea can be implemented inappropriately for the specific project; it can be invalidated by another effort that you may not even be aware of; the timing may be wrong, and a dozen other reasons can conspire against you.

Those responsible for maintaining the code have the best perspective, so engage them early. That is not to say they are always right, and you may have to make your case regardless, but this is a discussion better had before you write the code, not after. This is one of the most frustrating paradoxes in open-source: all too often it is your most engaged fans that fall into this trap, spending their nights and weekends working on a massive contribution, only to be disappointed when the maintainer flat out rejects their work.

Code reviews are hard: start early

Next time, before you dive into the editor, open a discussion, outline your problem and solution and offer to do the work. It shouldn't take more than a paragraph to explain the goal - if you need an essay, that's a sign already that you need to break it up, or the problem is not yet well defined. If the project or the change is large, pick a specific reviewer and go over the design. Ask for help and input early, and magically, you will find your code getting "pulled" (merged) on schedule.

Code reviews are hard. In fact, as it turns out, the effectiveness of a code-review drops dramatically after 200-400 lines of code, and the suggested inspection rate is at most 300-500 lines an hour. Keep this in mind, break down large commits into small, manageable chunks. As a rule of thumb, shoot for half an hour of review, or less than 200 lines with good and clear (not clever) code. If there are multiple commits, then provide a roadmap, and get feedback on each piece as it comes together.

Good work gets "pulled" not "pushed"

There is nothing more frustrating than to read the threads, or even worse, be on the receiving end of this experience gone wrong. The contributor is wounded because they are not recognized, and the maintainer is caught in a dilemma: they want to build and foster their community, but they have to dig-in against their biggest fans. There is no winner here and neither side is to blame.

Next time, before you dive into the editor, open a discussion, outline your problem and solution, and offer to do the work - don't "push" the pull request, and don't work in the dark.

Tracking Google Analytics events in development environment with GoogleAnalyticsProxy »

Created at: 01.11.2009 21:55, source: Robby on Rails, tagged: Ruby on Rails PostgreSQL javascript analytics kpi googleanalytics events proxy opensource github prototype

As mentioned in a recent article¹, I’ve been diving deep into Google Analytics lately while working on a few client projects. We’re aiming to use much more of the features of Google Analytics and have been hitting some roadblocks with the development versus production application environments. Once you begin to dive into event tracking and AJAX-driven goal conversions, relying on just the sample code that Google Analytics provides you is going to result in you looking at a handful of JavaScript errors.

another example from the firebug javascript console…

We see JavaScript errors like this because we don’t load the google analytics code in our development environments. As you can see, we are only loading this in our production environment.

  <% if RAILS_ENV == 'production' -%>
    <!--// Google Analytics //-->
    <script type="text/javascript">
    var gaJsHost = (("https:" == document.location.protocol) ? "https://ssl." : "http://www.");
    document.write(unescape("%3Cscript src='" + gaJsHost + "google-analytics.com/ga.js' type='text/javascript'%3E%3C/script%3E"));
    </script>
    <script type="text/javascript">
    var pageTracker = _gat._getTracker("UA-XXXXXX-1");
    pageTracker._trackPageview();
    </script>
  <% end -%>

To track an event with Google Analytics, you’d need to trigger something like:

  pageTracker._trackEvent('Button', 'Click', 'Get in touch');

As you can see from our code earlier, in development, the pageTracker variable isn’t defined and that’s why we’re getting those JS errors. We also don’t want to add conditionals everywhere in our application to check if we’re in development or production environment.. as that’d just make our views uglier than they need to be. So, I decided that I’d create a proxy class in JavaScript that would allow us to trigger _trackEvent() and _trackPageview() and handle it appropriately.

This class works with the following logic:

• if google analytics is loaded, pass the parameters to the real pageTracker
• if google analytics is NOT loaded, output the information to console.log() for debugging purposes

For example, on a gallery on our web site… we track when people navigate next and/or previous through the photos. In our development environment, I can watch the JavaScript console output the following:

And in our production environment, we can see that this was sent to Google Analytics.

We’re able to do this by initializing the GoogleAnalyticsProxy class and calling these functions through it. For example:

  _gap = new GoogleAnalyticsProxy();
  _gap._trackEvent('Video', 'Play', 'Homepage video');
  _gap._trackEvent('Video', 'Pause', 'Homepage video');
  _gap._trackEvent('Button', 'Click', 'Call to action X');

You’ll see that we’re just calling _gap versus pageTracker. We then replace all the instances of pageTracker (except where it is defined in the google analytics code block they provide you). You’ll find this located near the bottom of our application.html.erb file.

<% if RAILS_ENV == 'production' -%>
  <!--// Google Analytics //-->
  <script type="text/javascript">
  var gaJsHost = (("https:" == document.location.protocol) ? "https://ssl." : "http://www.");
  document.write(unescape("%3Cscript src='" + gaJsHost + "google-analytics.com/ga.js' type='text/javascript'%3E%3C/script%3E"));
  </script>
  <script type="text/javascript">
  var pageTracker = _gat._getTracker("UA-XXXXXX-1");
  pageTracker._trackPageview();
  </script>
<% end -%>

<script type="text/javascript">
  var _gap = new GoogleAnalyticsProxy();
</script>

We now have _gap available throughout our project and can call _trackEvent() and _trackPageview() with it. Note: You can use any JS variable name that you want, _gap is just what I went with.

Get GoogleAnalyticsProxy

I’ve gone ahead and tossed this small JavaScript class (known as GoogleAnalyticsProxy) on Github for your enjoyment. I have some more articles in the works that will show you some tips for how to make the most of Google Analytics. If you have any questions and/or ideas for related article topics, don’t hesitate to let me know.

¹ Tracking AJAX-driven events in Ruby on Rails for Google Analytics conversion goals