Re: [PATCH 1/2] bundle: Accept prerequisites without commit messages

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

 



Lukas Fleischer <git@xxxxxxxxxxxxxx> writes:

> While explicitly stating that the commit message in a prerequisite
> line is optional, 

Good spotting.  What e9ee84cf28b6 (bundle: allowing to read from an
unseekable fd, 2011-10-13) meant to say was the SP was optional but
we want to allow a tip bundled to have a commit without any commit
log message (hence making it "optional"), and the check you are
looking at does try to enforce.  What was buggy was that the
comparison did not take into account that the codepath earlier
called rtrim() on it, stripping "-<object name>SP<eol>" of the
trailing SP it wants to look for.

As to the order of comparison to match the order on the number line,
e.g. write "0 < something" or "negative < 0" to let readers more
easily visualize in what relation on the number line the quantity of
each side of the comparison stands, here is a reference to a long
and amusing thread:

  http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=3912

We never had any official guideline on which to use.  A patch that
changes an existing "b > a" to "a < b" (this directoin only) because
of the above thread is _not_ "a patch to fix coding style", and is
very much unwelcome.

Similarly, a patch that changes an existing "a < b" to "b > a" (or
vice versa) only because the author thinks it is easier to read is
not welcomed.

A switch done as a part of other meaningful rewrite is not a big
enough deal to make a fuss over, though.  Your patch falls into this
category, I think.

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