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]

 



Christian Couder <christian.couder@xxxxxxxxx> writes:

> 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,...":

Don't.

If you have four things there, and two are related, you would do

    /* used for --distim option */
    int distim_weight;
    int distim_target;

    int unrelated;

    int another;

And the first two are shown clearly to be related and unrelated to
the other ones.  That sound nice in theory, but think what you would
do when you want to comment further on one of the related ones.  You
would likely do this:

    /* used for --distim option */
    int distim_weight;
   -int distim_target;
   +int distim_target; /* which dosh to distim? */

    int unrelated;

For the same reason, if you are to comment on one of the ungrouped
things, you would do this:


   -int unrelated;
   +int unrelated; /* this is not used for --distim */

    int another;

Notice that a blank between unrelated and another is not helping
anything?  There is no reason to keep blanks in between each and
every unrelated things.

See how "struct diff_options" is done for a better example.  A blank
line every once in a while that roups related things together is
good.  Blindly applying it to all groups of one defeats the benefit
by making the result harder to read.

--
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]