On Sat, Jun 8, 2013 at 4:32 PM, Célestin Matte
<celestin.ma...@ensimag.fr> wrote:
> Le 08/06/2013 02:39, Eric Sunshine a écrit :
>> On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
>> <celestin.ma...@ensimag.fr> wrote:
>>> - strings which don't need interpolation are single-quoted for more clarity 
>>> and
>>> slight gain of performance
>>> - interpolation is preferred over concatenation in many cases, for more 
>>> clarity
>>> - variables are always used with the ${} operator inside strings
>>> - strings including double-quotes are written with qq() so that the quotes 
>>> do
>>> not have to be escaped
>>
>> Distinct changes could (IMHO) be split into separate patches for easier 
>> review.
>
> This commit is a real pain to cut into 3 distinctive ones. Is this
> really necessary?
> I will do it if it is, of course.

[I think you meant that it would be split into 4 patches.]

The final decision is up to the submitter (you) and the people signing
off on and accepting the patch (Matthieu and Junio).

Speaking merely as a person reviewing the patch series, I can say that
mixing conceptually unrelated changes into a single patch makes review
more onerous since it requires repeatedly switching mental gears
(often from line to line or even within a single line). Patches
involving simple "mechanical" changes typically are easy to review,
even when the patches are lengthy. In this case, however, that length
coupled with the mental gear switching, makes the review process more
burdensome that it need be.

Even if you decide ultimately not to bother splitting the patch,
ease-of-review and one-conceptual-change-per-patch are useful notions
to keep in mind for future submissions.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to