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

Re: [RFC PATCH v2 0/2] suppress trailing whitespace on empty "notes" lines

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Aug 30, 2021, 16:45 UTC
Message-ID
<CAPig+cRFCtVPoBfMswiO1i6bMw_BUess9q2pto_v4dzHp9X2RA@mail.gmail.com>
In-Reply-To
<RFC-cover-v2-0.2-00000000000-20210830T103913Z-avarab@gmail.com>

On Mon, Aug 30, 2021 at 6:47 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 17 quoted lines
> Side note:
>
>     I'm generally trying to see if just sending a "proposed vX" is
>     more productive for everyone than patch feedback effectively
>     describing it in prose. I don't mean for this thing to be picked
>     up as-is by Junio without the consent of the submitter, and don't
>     have any desire to "pick up" the series myself.
>
> I really like the end goal of
> <20210830072118.91921-1-sunshine@sunshineco.com> series, but this
> seems like a more straightforward way to get to that goal.
>
> I.e. the original 1/3 and 2/3 starts out by making the tests
> whitespace-independent. If we just skip that 1/3, and then in 3/3
> tweak the relevant failing tests for the code change we won't even
> need a new test, all the existing tests previously made
> whitespace-independent in 1/3 will assert this new behavior.

It probably won't surprise you that this fix to `notes` started out as a single patch which made the change to `notes.c` and adjusted the existing tests to account for it. In particular, my original changes to t3301 were exactly the same changes you made (i.e. merely dropping the empty-line `${indent}` from the few necessary places). I wasn't happy about the additional complexity I had to add to t3303 and t9301 to continue plucking the notes out of the default git-log output, thus simplified by making those tests less brittle. That, of course, deserved its own patch. I wavered quite a bit about whether to make t3301 less brittle too, or to simply apply the minimal changes which I had already made (and which you made independently). Eventually, I decided to split that out as a brittle-fixing patch, as well, to better future-proof it, but perhaps that's terribly important.

I don't have strong feelings between my v1 and your v2 of this series, and would be happy to see Junio pick up either version.

Previous: Ævar Arnfjörð BjarmasonNext: Eric Sunshine
Message 23 of 25 in “suppress trailing whitespace on empty "notes" lines”
  1. 0/3 suppress trailing whitespace on empty "notes" linesEric Sunshine, Aug 30, 2021
  2. 1/3 t3301: tolerate minor notes-related presentation changesEric Sunshine, Aug 30, 2021
  3. Junio C HamanoAug 30, 2021
  4. Eric SunshineAug 30, 2021
  5. 2/3 t3303/t9301: make `notes` tests less brittleEric Sunshine, Aug 30, 2021
  6. 3/3 notes: don't indent empty linesEric Sunshine, Aug 30, 2021
  7. Junio C HamanoAug 30, 2021
  8. Eric SunshineAug 30, 2021
  9. Junio C HamanoAug 30, 2021
  10. Eric SunshineAug 30, 2021
  11. Eric SunshineSep 10, 2021
  12. Eric SunshineSep 10, 2021
  13. Junio C HamanoSep 10, 2021
  14. Eric SunshineSep 10, 2021
  15. Ævar Arnfjörð BjarmasonSep 11, 2021
  16. Eric SunshineSep 11, 2021
  17. Ævar Arnfjörð BjarmasonSep 11, 2021
  18. Eric SunshineSep 12, 2021
  19. Junio C HamanoSep 12, 2021
  20. 0/2 suppress trailing whitespace on empty "notes" linesÆvar Arnfjörð Bjarmason, Aug 30, 2021
  21. 1/2 t3303/t9301: make `notes` tests less brittleÆvar Arnfjörð Bjarmason, Aug 30, 2021
  22. 2/2 notes: don't indent empty linesÆvar Arnfjörð Bjarmason, Aug 30, 2021
  23. Eric SunshineAug 30, 2021
  24. Eric SunshineAug 30, 2021
  25. Junio C HamanoAug 30, 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.