Elijah Newren <newren@xxxxxxxxx> writes:
> On Fri, Dec 18, 2020 at 5:41 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>>
>> Elijah Newren <newren@xxxxxxxxx> writes:
>>
>> > On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>> >>
>> >> Put open curly brace on its own line
>> >>
>> >> Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx>
>> >> ---
>> >> diff-merges.c | 36 ++++++++++++++++++++++++------------
>> >> 1 file changed, 24 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/diff-merges.c b/diff-merges.c
>> >> index cba391604ac7..0165fa22fcd1 100644
>> >> --- a/diff-merges.c
>> >> +++ b/diff-merges.c
>> >> @@ -2,7 +2,8 @@
>> >>
>>
>> [...]
>>
>> >>
>> >> -void diff_merges_set_dense_combined_if_unset(struct rev_info *revs) {
>> >> +void diff_merges_set_dense_combined_if_unset(struct rev_info *revs)
>> >> +{
>> >> if (!revs->combine_merges)
>> >> set_dense_combined(revs);
>> >> }
>> >> --
>> >> 2.25.1
>> >>
>> >
>> > But...didn't you add all these functions yourself earlier in the
>> > series?
>>
>> Yes, I did indeed and somehow picked wrong style from one of the
>> occurrences of this style in the existing Git codebase.
>>
>> > Why didn't you split this patch up and squash it into the
>> > relevant previous patches?
>>
>> When Junio noticed and pointed to this deficiency, I asked him if I
>> should fix all the series from the start, or it'd be OK to use fixup
>> commit. As he didn't answer and nobody else commented either, I opted
>> for the latter.
>>
>> I can still do it if it's that essential, but I'd prefer not to, to
>> avoid both the hand-work and causing entire series to change. The
>> problem is that there were code movements in the series, so such a fix
>> to earlier patches would cause conflicts down the commits chain, to be
>> resolved by hand.
>
> That's what add -p and interactive rebase is for. :-) Code is read
> more than it is written, so it's important to get things clean. And
> not just for immediate reviewers, but for people who look at it later.
I do know how to do it, but I'd still prefer it to be accepted in its
current form, as I don't see this particular case being that important
to justify rewriting of all the series. I'll go through the pain if
it's a show-stopper though.
Thanks,
-- Sergey
