Re: [PATCH v2 16/22] git-remote-mediawiki: Modify strings for a better coding-style

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 9, 2013, 04:44 UTC
Message-ID: <CAPig+cT5YQb9ZTX=x0bSv6Y=stWpR77wKt0xt-CuFhMidc3Ocw@mail.gmail.com>
In-Reply-To: <51B394E5.8040104@ensimag.fr>

On Sat, Jun 8, 2013 at 4:32 PM, Célestin Matte <celestin.matte@ensimag.fr> wrote:

Show 15 quoted lines

> Le 08/06/2013 02:39, Eric Sunshine a écrit :
>> On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
>> <celestin.matte@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.