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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
