From: Eric Sunshine <sunshine@sunshineco.com>
To: ZheNing Hu <adlternative@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
Denton Liu <liu.denton@gmail.com>,
ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
Git List <git@vger.kernel.org>
Subject: Re: [PATCH v2] format-patch: allow a non-integral version numbers
Date: Wed, 3 Mar 2021 22:27:25 -0500 [thread overview]
Message-ID: <CAPig+cTncEC4njnu+FB9tKwu20xi_UuL4TWW3_zD3drD3fyrHw@mail.gmail.com> (raw)
In-Reply-To: <CAOLTT8RdXC+KQNupU1TQdPh-tQO+syd6WJe85GzieE3uWt2ibA@mail.gmail.com>
On Wed, Mar 3, 2021 at 9:08 PM ZheNing Hu <adlternative@gmail.com> 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.
next prev parent reply other threads:[~2021-03-04 3:29 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-25 16:15 [PATCH] format-patch: allow a non-integral version numbers ZheNing Hu via GitGitGadget
2021-02-25 17:56 ` Eric Sunshine
2021-02-27 7:00 ` ZheNing Hu
2021-02-25 18:13 ` Junio C Hamano
2021-02-27 7:30 ` ZheNing Hu
2021-03-17 11:46 ` Đoàn Trần Công Danh
2021-03-17 19:17 ` Junio C Hamano
2021-03-01 8:40 ` [PATCH v2] " ZheNing Hu via GitGitGadget
2021-03-03 3:44 ` Junio C Hamano
2021-03-03 9:02 ` Denton Liu
2021-03-04 0:54 ` Junio C Hamano
2021-03-04 2:08 ` ZheNing Hu
2021-03-04 3:27 ` Eric Sunshine [this message]
2021-03-04 8:41 ` Denton Liu
2021-03-04 12:12 ` [PATCH v3] " ZheNing Hu via GitGitGadget
2021-03-04 12:49 ` Denton Liu
2021-03-05 4:56 ` ZheNing Hu
2021-03-05 7:10 ` [PATCH v4] " ZheNing Hu via GitGitGadget
2021-03-15 23:41 ` Eric Sunshine
2021-03-16 5:48 ` ZheNing Hu
2021-03-16 6:15 ` Eric Sunshine
2021-03-17 17:27 ` Junio C Hamano
2021-03-16 8:25 ` [PATCH v5] " ZheNing Hu via GitGitGadget
2021-03-16 23:36 ` Eric Sunshine
2021-03-17 2:05 ` ZheNing Hu
2021-03-18 6:00 ` [PATCH v6] " ZheNing Hu via GitGitGadget
2021-03-19 6:00 ` Eric Sunshine
2021-03-19 7:25 ` ZheNing Hu
2021-03-19 11:21 ` [PATCH v7] " ZheNing Hu via GitGitGadget
2021-03-19 16:01 ` Junio C Hamano
2021-03-20 3:08 ` ZheNing Hu
2021-03-19 17:28 ` Junio C Hamano
2021-03-20 3:04 ` ZheNing Hu
2021-03-20 14:56 ` [PATCH v8] " ZheNing Hu via GitGitGadget
2021-03-20 19:55 ` Junio C Hamano
2021-03-21 2:45 ` ZheNing Hu
2021-03-21 4:05 ` Eric Sunshine
2021-03-21 5:45 ` Junio C Hamano
2021-03-21 5:54 ` Eric Sunshine
2021-03-24 8:46 ` Denton Liu
2021-03-21 7:22 ` ZheNing Hu
2021-03-21 9:00 ` [PATCH v9] " ZheNing Hu via GitGitGadget
2021-03-23 5:31 ` Eric Sunshine
2021-03-23 6:42 ` Junio C Hamano
2021-03-23 8:53 ` ZheNing Hu
2021-03-23 9:16 ` ZheNing Hu
2021-03-23 11:12 ` [PATCH v10] " ZheNing Hu via GitGitGadget
2021-03-24 3:58 ` Eric Sunshine
2021-03-24 4:43 ` ZheNing Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPig+cTncEC4njnu+FB9tKwu20xi_UuL4TWW3_zD3drD3fyrHw@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=adlternative@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=liu.denton@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).