[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