Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'

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

 



On Tue, Apr 26, 2016 at 10:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
>> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> ---
>>  builtin/apply.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 506357c..c45e481 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -57,6 +57,8 @@ struct apply_state {
>>       int unidiff_zero;
>>
>>       int update_index;
>> +
>> +     int unsafe_paths;
>>  };
>
> Having said
>
>     I like the way this series moves only a few variables at a time to
>     limit the scope of each step.
>
> it gets irritating to see all these unnecessary blank lines in the
> structure definition added by each step, which would mean all of
> these patches need to fix them in the next reroll.

The reason I added some blank lines is that I moved comments that were
all in one block at the top.
I moved each comment near the related variables and sometimes a
comment is related to 2 variables, like in the extract below the
comment that starts with "For "diff-stat" like behaviour,...":

------------------------------------------------------

    /* --index updates the cache as well. */
    int check_index;

    int unidiff_zero;

    int update_index;

    int unsafe_paths;

    int line_termination;

    /*
     * For "diff-stat" like behaviour, we keep track of the biggest change
     * we've seen, and the longest filename. That allows us to do simple
     * scaling.
     */
    int max_change;
    int max_len;

    /*
     * Various "current state", notably line numbers and what
     * file (and how) we're patching right now.. The "is_xxxx"
     * things are flags, where -1 means "don't know yet".
     */
    int linenr;

------------------------------------------------------

If I remove all the blank lines, I think it will make it harder to
understand which comment belong to which variable(s).

Maybe a compromise would be to just remove blank lines between the
variables that don't have any related comment like "unidiff_zero",
"update_index", "unsafe_paths" and "line_termination" above.
--
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]