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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 8, 2013 at 4:32 PM, Célestin Matte
<celestin.matte@xxxxxxxxxx> wrote:
> Le 08/06/2013 02:39, Eric Sunshine a écrit :
>> On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
>> <celestin.matte@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]