Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

Jay Soffian <jaysoffian@gmail.com> writes:

Different people have different priorities and judging criteria.

To some authors, code is the only thing that matters and they say a crappy commit log message is Ok if the code works. I actually do not even read the patch if the commit log message does not clearly express what problem the author is trying to address, because I always have other patches to attend to, and if the author cannot formulate his thought clearly in the commit log message, it is likely that the code doesn't, either, and even if it does, the code speaks only about how it does what it does, and not much about _why_ it is so, which is the most important thing down the road.

Some people comment more on readability of the patch and format of the code while some others let them pass and concentrate on performance and correctness.

All of them are important, and reviewers, together with the original author, complement each other's weakness to work toward a better system.

The first thing to learn is to tell the difference between "you didn't cut that wart off to my standards, unless you can do it better, I'd rather have the wart." and "you claim you have cut the wart off, but that's minor compared to the wart you are missing here which can be cut at the same time without too much effort; let's do so at the same time before we all forget".

The review comments I read on this list are more often the latter, but I can certainly understand some authors, being so excited about and fond of his own creation, mistake it as the former. Two tricks I learned to avoid that trap while working on git, and especially when git was still young and I was a contributor, was to sleep on things, and to always consider the possibility that I _might_ be wrong. The latter takes some practice, and I admit I still haven't mastered it yet, but I am trying.