Thread (4 messages) 4 messages, 4 authors, 2021-03-04

Re: [PATCH v2] format-patch: allow a non-integral version numbers

flat view

From: Eric Sunshine <hidden>
Date: 2021-03-04 03:29:16

On Wed, Mar 3, 2021 at 9:08 PM ZheNing Hu [off-list ref] wrote:
What we are arguing now is whether it is necessary to add
"aginst v<previous_count>" to the patch when the non-integer version
number + rangediff/interdiff is required. Denton's point of view may be
similar to that of Eric before.
Yes, it sounds as if Denton and I share the same point of view.
Here are my personal thoughts:

Of course this `previous count` can be used in a very small range, but
I think it
 doesn't hurt to keep it, because even if you don't use it, `format
patch` will still
output "Range-diff", which will not break any known functions. It can
only be said
that `previous count` provides an option for submitters to know the
previous version
 for reviewers. In this regard, I agree with Junio's point of view.
I'm not outright opposed to supporting non-numeric, non-integer
reroll-counts, but I also don't see a big need for it. As mentioned
earlier, Denton is the only person I recall who sends fractional
re-rolls, so it's not obvious that there is a big advantage to adding
such support and complicating the code just for one person. Also, when
Denton does send fractional re-rolls, he typically does so for just a
single patch out of a longer series, and he doesn't (I think) provide
a range-diff or interdiff for the patch. So, for Denton's intended
use-case, this entire discussion about "Range-diff against v$V" and
"Interdiff against v$V" seems superfluous. That is, the simple logic:

    if reroll_count specified and is integer:
        s = "Range-diff against v${reroll_count -1}:"
    else
        s = "Range-diff:"

satisfies Denton's case without the complication of adding a
--previous-count switch. This probably explains why Denton doesn't see
a need for the extra complexity of --previous-count.

So, some ways forward are:

(1) drop this topic altogether since it so far seems of interest to
only a single person (Denton) -- nobody else has asked for it

(2) support non-integer reroll-count but just use the simple logic
shown above for constructing the "Range-diff/Interdiff against"
header; this leaves the door open for Junio's idea(s) of allowing the
user to specify --previous-count and automatically determining the
previous reroll-count by scanning a directory

(3) continue refining the changes made by this patch until reviewers
are happy with it (which might take a few more re-rolls)

I lean toward #1, but wouldn't be opposed to #2 or #3 either.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help