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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 17, 2021, 17:27 UTC
Message-ID: <xmqqtup9y9kj.fsf@gitster.g>
In-Reply-To: <CAPig+cRkGvKU8b2vauUZ4=EtTj4DU2e9k0HciTVEz-6-P6X5KQ@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 7 quoted lines

> Yes, it is a very minor modification to diff_title(), changing
> `reroll_count` from int to string.
>
> Moving the parsing into diff_title() also allows you to use the
> simpler name `reroll_count` for the string variable in cmd_fmt_patch()
> rather than the longer `reroll_count_string`; just change
> `reroll_count` from int to string.

It does clarify that the "string" version is the authoritative and features that depend on being able to learn what the previous iteration is is available only it is given an integral version number.

It made a ton of sense when the code allowed an explicit "previous iteration" to be given by the user also as a string to compute the previous version with strtol_i() dance, so that diff_title() can be a mere consumer of both string versions (the current one and the previous one) without having to worry about how the previous one is computed. But without the "previous iteration" string carried through the codebase as another first-class citizen just like the "current iteration" string, it does not.

I actually think you would regret not allowing the explicit "previous iteration" string from the command line and make it another first-class citizen, but that is a different story.