git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite

From
Junio C Hamano <gitster@pobox.com>
Date
May 6, 2021, 22:33 UTC
Message-ID
<xmqqv97vwkzo.fsf@gitster.g>
In-Reply-To
<20210506165102.123739-1-firminmartin24@gmail.com>
Firmin Martin <firminmartin24@gmail.com> writes:
Show 6 quoted lines
> Particulary, this behaviour could be awkward in the following
> hypothetical situations:
>
> * The user can easily erase a cover letter coming from prior versions or
> another patch series by reusing an old command line (e.g. autocompleted
> from the shell history).

"prior versions" implies that the user is better off using -v$n where $n is the number greater than the one used for the prior iteration by one, and there won't be any overwriting, so this is not a very compelling use case.

But the next one is real.
Show 5 quoted lines
> * Assuming that the user is writing a cover letter and realizes that
> small changes should be made. They make the change, amend and
> format-patch again to regenerate patches. If it happens that they use
> the same command again (e.g. with --cover-letter), the cover letter
> being written is gone.

Yes, after preparing, say, -v2, but before sending them out, it is very plausible that proofreading of your own patches may make you realize more issues in the series, which may make you go back to your commits, "rebase -i" to improve them and re-run "format-patch -v2".

We do want to encourage such careful preparation of your patch series before sending it out, and we want to support it well with our tools. Preventing overwriting of the cover (which will have the same filename, with the same v2- prefix) is very valuable here.

There is another thing that I suspect people may find irritating in the same workflow. If you fix the commit title while "rebase -i" to polish your v2 patch, it would result in a different filename from the original v2, so you'd end up with something like

    v2-0000-cover-letter.patch
    v2-0001-thes-forny-change.patch
    v2-0001-this-phoney-change.patch
    v2-0002-another-sample-change.patch

while redoing a two-patch series. The "thes-forny" thing is a leftover from the first "format-patch -v2" run, you fixed typoes with "rebase -i" after a self-review and other three files have the result of the second "format-patch -v2" run. You need a way to somehow exclude that stale file when driving send-email; in other words, before running

    git send-email v2-*.patch

you would want to move away v2-0001-thes-forny-change.patch that no longer is part of the series. I wonder if format-patch can help by looking at the output directory before writing its output and move the old files away, say, to "old-v2-*.patch" or something? That incidentally would solve your "files getting overwritten is irritating" issue at the same time.

Coming back to the topic of cover letter, even when there is no risk of ovetwriting, there is another thing we may want to improve to help our users. Suppose you are preparing your v2 patch after sending out the v1. The cover letter we generate for v2 will have the same **BOILERPLATE** placeholders that need to be filled from scratch. As many things you wrote for the cover letter in the previous round should be reusable, even though the list of titles of the patch should be generated afresh, it would be nice if format-patch can carry forward what you wrote in the cover letter for the v1 iteration to the cover letter for this v2 iteration.

And when we have such a "reuse description in the existing cover letter" support, the value of "don't overwrite" knob will mostly go away. Instead of failing the command by refusing to overwrite, you can read what is in the existing cover-letter that is about to be overwritten, combine the hand-written description with the material automatically generated, and ovewrite the existing file, and as long as you do a good job of preserving, nobody would complain that you overwrote the file.

Previous: Firmin MartinNext: Firmin Martin
Message 25 of 34 in “format-patch: introduce --confirm-overwrite”
  1. 0/8 format-patch: introduce --confirm-overwriteFirmin Martin, May 6, 2021
  2. 1/8 compat/terminal: let prompt accept input from pipeFirmin Martin, May 6, 2021
  3. Junio C HamanoMay 6, 2021
  4. Jeff KingMay 7, 2021
  5. Junio C HamanoMay 7, 2021
  6. Firmin MartinMay 10, 2021
  7. Jeff KingMay 10, 2021
  8. Junio C HamanoMay 11, 2021
  9. Jeff KingMay 11, 2021
  10. Junio C HamanoMay 11, 2021
  11. Jeff KingMay 11, 2021
  12. 2/8 format-patch: confirmation whenever patches existFirmin Martin, May 6, 2021
  13. Junio C HamanoMay 6, 2021
  14. Firmin MartinMay 10, 2021
  15. Junio C HamanoMay 10, 2021
  16. Firmin MartinMay 11, 2021
  17. 3/8 format-patch: add config option confirmOverwriteFirmin Martin, May 6, 2021
  18. 4/8 format-patch: add the option --confirm-overwriteFirmin Martin, May 6, 2021
  19. 5/8 t4014: test patches overwrite confirmationFirmin Martin, May 6, 2021
  20. 6/8 t4014: fix tests overwriting cover letter in silentFirmin Martin, May 6, 2021
  21. 7/8 doc/format-patch: describe --confirm-overwriteFirmin Martin, May 6, 2021
  22. Bagas SanjayaMay 7, 2021
  23. Firmin MartinMay 10, 2021
  24. 8/8 config/format: describe format.confirmOverwriteFirmin Martin, May 6, 2021
  25. Junio C HamanoMay 6, 2021
  26. Firmin MartinMay 11, 2021
  27. Felipe ContrerasMay 7, 2021
  28. Denton LiuMay 7, 2021
  29. Firmin MartinMay 11, 2021
  30. Felipe ContrerasMay 11, 2021
  31. Felipe ContrerasMay 11, 2021
  32. Sergey OrganovMay 7, 2021
  33. Firmin MartinMay 11, 2021
  34. Ævar Arnfjörð BjarmasonMay 10, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.