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.