Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison

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

 



On 06/08/2016 07:17 PM, Junio C Hamano wrote:
Here is an illustration of an organization that I think would be
easier to read, and would result in a more readable patch when
modification is made on top.  The first two hunks collapse the
overall "setup" steps that appear as three separate tests into a
single "setup" test.  The last hunk that begin at -83/+79 collapses
a logically-single test that is split across three into one, and
makes the order of things done in the test to (1) set an
expectation, (2) execute the command and (3) compare the result with
the expectation.

I totally agree. (1), (2) and (3) aren't even always in that order, some tests are very confusing.

I am not going to commit this myself, because I do not want to
create conflicts with the change your topic is trying to do, and
besides, almost all the remainder of the tests follow "one logical
test split into three" pattern and need to be corrected before this
"illustration" can become a real patch.

I do not mind if you take it and complete it as a preliminary
clean-up step in your series; or you can "keep it in mind, but
ignore it for now", in which case this can be a "low hanging fruit"
somebody else, hopefully somebody new to the development community,
can use to dip their toes ;-)

As said in my other reply, I will put this one aside for now, but t9001 definitely deserves its own cleanup patch series.
--
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]