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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 20, 2021, 19:55 UTC
Message-ID: <xmqqv99lk3c7.fsf@gitster.g>
In-Reply-To: <pull.885.v8.git.1616252178414.gitgitgadget@gmail.com>

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

As the patch text itself (assuming that the vague "compared with the previous iteration" is acceptable) is getting solid, let's nitpick the proposed log message so that we can start considering a merge to 'next'.

I won't review the t/ part of the patch as I know others on CC are better at reviewing the tests than I am ;-)

> From: ZheNing Hu <adlternative@gmail.com>
>
> Usually we can only use `format-patch -v<n>` to generate integral
> version numbers patches,

The "only" sounds as if you are saying that "there is no tool other than 'format-patch -v' we can use", which might be technically true, but is not what you want to stress to your readers here. You are sayig that usually people only use integral version numbers.

> but sometimes a small fixup should be
> labeled as a non-integral version like `v1.1`, so teach `format-patch`
> to allow a non-integral version which may be helpful to send those
> patches.

Also, I do not think we want to encourage such use of fractional iteration numbers. We can merely enable, without saying it is a good idea or a bad idea, leaving judgment to each project that may or may not accept a patch versioned in such a way. So avoid "should be".

That makes the first part of the log message to be something like:

    The `-v<n>` option of `format-patch` can give nothing but an
    integral iteration number to patches in a series.  Some people,
    however, prefer to mark a new iteration with only a small fixup
    with a non integral iteration number (e.g. an "oops, that was
    wrong" fix-up patch for v4 iteration may be labeled as "v4.1").

    Allow `format-patch` to take such a non-integral iteration
    number.

> `<n>` can be any string, such as '3.1' or '4rev2'. In the case
> where it is a non-integral value, the "Range-diff" and "Interdiff"
> headers will not include the previous version.

This part is well written and can stay as-is.

Show 8 quoted lines

> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>     [GSOC] format-patch: allow a non-integral version numbers
>     
>     There is a small question: in the case of --reroll-count=<n>, "n" is an
>     integer, we output "n-1" in the patch instead of "m" specified by
>     --previous-count=<m>,Should we switch the priority of these two: let "m"
>     output?

Didn't I answer this question already?

Show 9 quoted lines

> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 3e49bf221087..e2c29a856451 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -221,6 +221,11 @@ populated with placeholder text.
>  	`--subject-prefix` option) has ` v<n>` appended to it.  E.g.
>  	`--reroll-count=4` may produce `v4-0001-add-makefile.patch`
>  	file that has "Subject: [PATCH v4 1/20] Add makefile" in it.
> +	 `<n>` may be a non-integer number.  E.g. `--reroll-count=4.4`

Is it on purpose that `<n>` here has an extra SP before it?

> +	may produce `v4.4-0001-add-makefile.patch` file that has
> +	"Subject: [PATCH v4.4 1/20] Add makefile" in it.
> +	`--reroll-count=4rev2` may produce `v4rev2-0001-add-makefile.patch`
> +	file that has "Subject: [PATCH v4rev2 1/20] Add makefile" in it.

I am not sure it is worth repeating the whole explanation already given to "v4" for two other. By just mentioning that the "v4" could be "v4.4" or "v4vis", the readers are intelligent enough to infer what you mean, I would think. Also be honest to readers by telling them what they lose if they use a non-integral reroll count.

	`<n>` does not have to be an integer (e.g. "--reroll-count=4.4",
	or "--reroll-count=4rev2" are allowed), but the downside of
	using such a reroll-count is that the range-diff/interdiff
	with the previous version does not state exactly which
	version the new interation is compared against.

or something like that, perhaps?

Thanks.