Re: [PATCH 2/2] graph API: avoid printing unnecessary padding before some octopus merges

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

 



On Sun, Jun 01, 2008 at 02:50:34PM -0700, Junio C Hamano wrote:
> Adam Simpkins <adam@xxxxxxxxxxxxxxxx> writes:
> 
> > diff --git a/graph.c b/graph.c
> > index 332d1e8..0531716 100644
> > --- a/graph.c
> > +++ b/graph.c
> > @@ -450,16 +450,18 @@ void graph_update(struct git_graph *graph, struct commit *commit)
> >  	 * it never finished its output.  Goto GRAPH_SKIP, to print out
> >  	 * a line to indicate that portion of the graph is missing.
> >  	 *
> > -	 * Otherwise, if there are 3 or more parents, we need to print
> > -	 * extra rows before the commit, to expand the branch lines around
> > -	 * it and make room for it.
> > +	 * If there are 3 or more parents, we may need to print extra rows
> > +	 * before the commit, to expand the branch lines around it and make
> > +	 * room for it.  We need to do this unless there aren't any branch
> > +	 * rows to the right of this commit.
> 
> Double negation like this is confusing, isn't it?
> 
> "We do not have to do this if there isn't any branch row to the right of
> this commit" may be better.  "We need to do this only if there is a branch
> row (or more) to the right of this commit" would probably be better.

Yes, I agree it is less confusing without the double negation.  Your
second choice of wording sounds best.

How do you prefer to fix simple things like this?  Do you want to just
apply the fix yourself, or is it easier for you if I submit an amended
patch?

-- 
Adam Simpkins
adam@xxxxxxxxxxxxxxxx
--
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]

  Powered by Linux