[PATCH 2 of 8] diff: Move diffline to patch module
bisho (Guillermo)
bisho at fb.com
Thu Nov 15 11:38:57 CST 2012
Ok, I will split this patch in two, one for diffline move and other for
addmodehdr move.
On 11/14/12 13:31 , "Matt Mackall" <mpm at selenic.com> wrote:
>On Wed, 2012-11-14 at 14:35 -0600, Kevin Bullock wrote:
>> On Nov 14, 2012, at 2:19 PM, bisho (Guillermo) wrote:
>>
>> >
>> >>> The move of this function seems unrelated to the purpose of the
>>patch.
>> >> Probably because it didn't need to be a top-level function.
>> >
>> >>> Also, why are we making it public?
>> >> It's not public, it's a function nested in another.
>> >
>> > Exactly, is not public, and I moved it within this patch because
>>diffline
>> > and that function follow the same behavior, they are helpers of
>>trydiff,
>> > so makes sense to move them together and nest them under trydiff.
>>
>>
>> Okay, makes sense.
>
>Kevin is of course absolutely right to point out that this patch doesn't
>match its description and apparently includes unrelated changes.
>
>This patch either needs a rewritten description or to be split into two
>patches. In general, we much prefer the latter. In most cases, the
>improved odds of smaller patches getting through the review process in
>one round-trip far outweigh any efficiency gained by combining them into
>larger patches (as evidenced by the five messages in this thread).
>
>--
>Mathematics is the supreme nostalgia of our time.
>
>
More information about the Mercurial-devel
mailing list