On Wed, Mar 3, 2021 at 9:08 PM ZheNing Hu <adlternative@xxxxxxxxx> 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.
