Friday, January 25, 2008

After 8 months, no longer a Noogler!

Today, I've got my readability in the last major Google language - Python, so I am now officially a full-fledged member of the engineering team! The three others that I already had were C++ (well, obviously!), Java, and JavaScript, although in actuality I've got them in the reverse order - first, JavaScript, and last - C++. And now, Python!

What's a "readability" you might ask?

Since Google is growing insanely fast, its engineering culture is defined rather tightly, which helps assimilating a lot of new people without dissimilating into a complete bedlam. There are several very rigid requirements with respect to writing source code:

  1. Every check-in (*) must be code reviewed.

  2. A person is only allowed to submit source in a language, or authorize submissions by other engineers, if (s)he was pre-approved by one of the vew experts in this specific language through a process called "readability review".

  3. If a person does not have "readability", he or she must seek a code review (and an approval) from people who have the said readability.

  4. All Google code is written in a specific style (which varies by the language, but not by product). The style guides are published, and all code must conform to them.

  5. All new code must be accompanied by tests (this one is occasionally but not so frequently violated).


(*) For non-engineers, to check-in means submitting code to the central database. All other engineers, build and test machines get it from there. When code is "checked-out", it's a work in progress residing only on an engineer's computer that may, for example, not compile or work. When code is checked in, other people get it, so it better work (and, in Google's case, be beautiful :-)).

Having a readability in the language therefore allows one to approve check-ins of the people who do not. It is less important for one's own check-ins, because they have to be reviewed anyway, and more important for the project, because there may be not enough (or even not at all) people who have readability in a language that the team is using.

For example, I am the only one who has readability in Java and JavaScript in our team, and it helps a lot - before I joined, they had to go and seek people from the outside who would kindly agree to review their code and approve the check-in.

To get readability, one has to produce a non-trivial body of code - usually several hundred lines - that uses non-trivial amount of Google platform, and language facilities. It should be written in an accepted dialect and testify to general command of language. Most readability reviewers require it to be accompanied by the unit tests.

After the code is written, it gets submitted to one of the few "readability reviewers" (when I joined readability team for JavaScript, there were only 4 people for the entire company, which was the reason I volunteered). The process often takes several weeks, and a lot of back and forth between the reviewer and reviewee. For example, my python code was almost completely redone by the end of the review, and it took over a month (although one week of which was the winter break).

Interestingly, most of the readability recommendations in my last three reviews were about test (in case of Java, just to write it - this is more or less my approach for my own readability reviewing), but in the case of C++ and Python the reviewers largely agreed with the main code base, but really took the tests to heart, making me more or less rewrite them completely.

Interesting, but not surprising. I found that if happens quite often that most of the arguments during code reviews at Google are about tests. The reason is of course that there are relatively few variations in which the main code can be written by a competent developer. Testing, however is different - a typical unit test covers maybe 10% of everything there is to test, and so the selection of the right 10% provides an excellent fodder for religious wars.

The other (strong) possibility is that we may be competent developers, but not competent testers :-)...

No comments: