| Message ID | cover.1713324598.git.dsimic@manjaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | format-patch: fix an option coexistence bug and add new --resend option | expand |
On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic <dsimic@manjaro.org> wrote: > This series fixes a bug that allows --rfc and -k options to be specified > together when running "git format-patch". This bug was introduced about > eight months ago, but it has remained undetected, presumably because of > lacking test coverage. While fixing this bug, also add a test that covers > this mutual exclusion, for future coverage. > > This series also adds --resend as the new option for "git format-patch" > that adds "RESEND" as a (sub)suffix to the patch subject prefix, which > eventually produces "[PATCH RESEND]" as the default patch subject prefix. > This subject prefix is commonly used on mailing lists to denote patches > resent after they had attracted no attention for a while. I'd recommend splitting this into two series, one which fixes the bug, and one which introduces the new feature. Otherwise, the bug fix is likely to be held hostage as reviewers bikeshed over the new feature and opine about whether such a feature is even desirable[*]. As a result, the bug fix may take much longer to get applied than if submitted as a standalone series. [*] For instance, my knee-jerk reaction is that we don't want to keep piling on these special-case flags each time someone wants their new favorite word as a lead-in to "PATCH". In addition to --rfc, and --resend, the next person might want --rfd or --tbd, etc. More palatable would be a general-purpose option which lets you specify the prefix which appears in front of "PATCH", but even that can be argued as unnecessary since we already have --subject-prefix.
Hello Eric, On 2024-04-17 08:02, Eric Sunshine wrote: > On Tue, Apr 16, 2024 at 11:33 PM Dragan Simic <dsimic@manjaro.org> > wrote: >> This series fixes a bug that allows --rfc and -k options to be >> specified >> together when running "git format-patch". This bug was introduced >> about >> eight months ago, but it has remained undetected, presumably because >> of >> lacking test coverage. While fixing this bug, also add a test that >> covers >> this mutual exclusion, for future coverage. >> >> This series also adds --resend as the new option for "git >> format-patch" >> that adds "RESEND" as a (sub)suffix to the patch subject prefix, which >> eventually produces "[PATCH RESEND]" as the default patch subject >> prefix. >> This subject prefix is commonly used on mailing lists to denote >> patches >> resent after they had attracted no attention for a while. > > I'd recommend splitting this into two series, one which fixes the bug, > and one which introduces the new feature. Otherwise, the bug fix is > likely to be held hostage as reviewers bikeshed over the new feature > and opine about whether such a feature is even desirable[*]. As a > result, the bug fix may take much longer to get applied than if > submitted as a standalone series. Thanks for the suggestion! I'll wait a couple of days for more feedback, and I'll then split the series. > [*] For instance, my knee-jerk reaction is that we don't want to keep > piling on these special-case flags each time someone wants their new > favorite word as a lead-in to "PATCH". In addition to --rfc, and > --resend, the next person might want --rfd or --tbd, etc. More > palatable would be a general-purpose option which lets you specify the > prefix which appears in front of "PATCH", but even that can be argued > as unnecessary since we already have --subject-prefix. Makes sense, but in that case accepting the --rfc option, back at the time, was actually some kind of a mistake, if you agree.
On Wed, Apr 17, 2024 at 2:07 AM Dragan Simic <dsimic@manjaro.org> wrote: > On 2024-04-17 08:02, Eric Sunshine wrote: > > [*] For instance, my knee-jerk reaction is that we don't want to keep > > piling on these special-case flags each time someone wants their new > > favorite word as a lead-in to "PATCH". In addition to --rfc, and > > --resend, the next person might want --rfd or --tbd, etc. More > > palatable would be a general-purpose option which lets you specify the > > prefix which appears in front of "PATCH", but even that can be argued > > as unnecessary since we already have --subject-prefix. > > Makes sense, but in that case accepting the --rfc option, back at the > time, was actually some kind of a mistake, if you agree. Possibly. It does happen that, in retrospect, some changes come to be viewed as mistakes. On the other hand, if --rfc existed before --subject-prefix was introduced, then --rfc would just be historic accretion rather than a mistake. (I didn't check which option came first.) At any rate, we probably want to be careful about piling on more special-cases without considering general-purpose solutions.
On 2024-04-17 08:23, Eric Sunshine wrote: > On Wed, Apr 17, 2024 at 2:07 AM Dragan Simic <dsimic@manjaro.org> > wrote: >> On 2024-04-17 08:02, Eric Sunshine wrote: >> > [*] For instance, my knee-jerk reaction is that we don't want to keep >> > piling on these special-case flags each time someone wants their new >> > favorite word as a lead-in to "PATCH". In addition to --rfc, and >> > --resend, the next person might want --rfd or --tbd, etc. More >> > palatable would be a general-purpose option which lets you specify the >> > prefix which appears in front of "PATCH", but even that can be argued >> > as unnecessary since we already have --subject-prefix. >> >> Makes sense, but in that case accepting the --rfc option, back at the >> time, was actually some kind of a mistake, if you agree. > > Possibly. It does happen that, in retrospect, some changes come to be > viewed as mistakes. On the other hand, if --rfc existed before > --subject-prefix was introduced, then --rfc would just be historic > accretion rather than a mistake. (I didn't check which option came > first.) > > At any rate, we probably want to be careful about piling on more > special-cases without considering general-purpose solutions. Yes, but the usability should also be taken into consideration. IOW, perhaps typing just --rfc or --resend is rather quick and usable, instead of having to use a general-purpose solution and type much more, or instead of having to create an alias.
This series fixes a bug that allows --rfc and -k options to be specified together when running "git format-patch". This bug was introduced about eight months ago, but it has remained undetected, presumably because of lacking test coverage. While fixing this bug, also add a test that covers this mutual exclusion, for future coverage. This series also adds --resend as the new option for "git format-patch" that adds "RESEND" as a (sub)suffix to the patch subject prefix, which eventually produces "[PATCH RESEND]" as the default patch subject prefix. This subject prefix is commonly used on mailing lists to denote patches resent after they had attracted no attention for a while. Dragan Simic (4): format-patch docs: avoid use of parentheses to improve readability format-patch: fix a bug in option exclusivity and add a test to t4014 format-patch: new --resend option for adding "RESEND" to patch subjects t4014: add tests to cover --resend option and its exclusivity Documentation/git-format-patch.txt | 9 +++++-- builtin/log.c | 16 +++++++++--- t/t4014-format-patch.sh | 41 ++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-)