Thread (1 message) 1 message, 1 author, 2021-08-30

Re: [RFC PATCH v2 0/2] suppress trailing whitespace on empty "notes" lines

From: Junio C Hamano <hidden>
Date: 2021-08-30 17:04:57

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
This is a review of Eric Sunshin's
[ref] series.

Side note:

    I'm generally trying to see if just sending a "proposed vX" is
    more productive for everyone than patch feedback effectively
    describing it in prose. I don't mean for this thing to be picked
    up as-is by Junio without the consent of the submitter, and don't
    have any desire to "pick up" the series myself.
It is impossible to read the rationale behind the change between v1
and "proposed v2" in such arrangement, simply because there is no
place in the "proposed v2" patch to write it down.  Proposed commit
log for it should not refer to what "v1" said, proposed postimage of
the patch should not refer to what "v1" did.  Worse, it is harder to
pick good bits from "proposed v2" and reject the others.

It is not a good way to give a feedback on "v1".  It does not help
the original author.

It certainly does not help the maintainer, who now has to read two
competing series, sort out the numbering mess.

I do not know if it helps other reviewers, but offhand I do not know
how it would, compared to the in-line comments on "v1", which is how
we usually do reviews.
    My review workflow is just applying the patches locally, fiddling
    with them, so it seems like the most straightforward and helpful
    thing to send the result of that local end-state, rather than
    describing the changes I made in prose, and expect the original
    submitter to reverse engineer that state if they're interested in
    trying it out locally themselves.
The end-state as an additional reference material attached as the
end note of the review may be helpful, but the in-line comments on
"v1" is a much better way to convey the reason why the change is
suggested, I would think.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help