codahale.com٭blog

Coda Hale lives in Berkeley, CA, where he writes about Ruby on Rails, usability, web design and development, and the occasional bit about bicycles.

Yay! My first accepted patch!

My first patch to Rails was accepted today. I was bored yesterday, and talking with Hampton, who was working on a really cool patch himself (which I’m sure he will explain in good time). We both noticed that four or five of the unit tests for ActiveRecord::Calculations were bombing out, and naturally did what Rails hackers do:

Hampton: Weird…. a fresh checkout is causing test errors…

Coda: On calculations?

Hampton: Something broke between 4485 and 4491

Hampton: However, the last to change to AR happened at 4484

Coda: Yeah, it’s broken. I’m looking into why.

Hampton: RACE!

81 seconds later…

Hampton: Found it!

Hampton totally schooled me. I had just gotten my debug code up and running when, just 81 seconds after the green light, Hampton found the problem. That’s why he gets mentioned in keynote addresses, I guess.

Yes, the newly added Enumerable#sum was mucking up ActiveRecord::AssociationProxy’s ability to pass off the sum method to ActiveRecord::Calculations#sum. Fixing it, however, was a hell of a lot harder, since neither of us had played around with ActiveRecord::AssociationProxy. Thankfully, I figured out that sum needed to be added to ActiveRecord::AssociationCollection, which is what’s actually used when ActiveRecord::Base#find returns an array of records instead of Array. I made the fix, verified that it passed all the unit tests, and submitted it. Nicholas Seckar commited the patch today.

So I finally got to give back to Rails, even if it was just a four or five lines of code. That makes me really happy. Rails is an open-source project, and it’s important that the community give back to it as much as possible. The greatest gift you can give a project like Rails is a patch for an outstanding ticket which conforms to the style guidelines, passes existing unit tests, and has new unit tests for any new code.

I hope they get around to my patch for eagerly-loaded associations not respecting the :order option. That’d be awesome.

8 comments »