Re: [PATCHv3 1/9] t7408: modernize style
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Aug 9, 2016, 23:06 UTC
- Message-ID
- <CAPig+cR+3KWDK5r5YW-Ew0=0jvkGRM7r1SJA3O_7+SDSNmXmeQ@mail.gmail.com>
- In-Reply-To
- <xmqqd1lhc05n.fsf@gitster.mtv.corp.google.com>
On Tue, Aug 9, 2016 at 1:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Stefan Beller <sbeller@google.com> writes: >> I originally thought that it may be easier to have 2 patches. >> This first one is very gentle and "obviously correct" as it only changes >> formatting and drops the directory changes. >> >> The second patch goes for renaming ans subtle style issues, >> combining some tests, so it is more likely to break. >> >> After this review, I consider using just one patch and do it all >> at once to not confuse the readers as otherwise I should reword >> the commit message with the rationale of doing it in two patches. > > FWIW, I would think your split between 1/ and 2/ were very sensible, > and have a slight preference for keeping them separate.
The review comment about renaming "current" to "actual" was made without having looked yet at patch 2. Having now seen patch 2, I agree with Junio that the existing split is preferable.
So, only the review comment about dropping space after '>' remains relevant.