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

Re: [PATCH 00/11] alternative unify appending of sob

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 29, 2012, 05:56 UTC
Message-ID
<7vvccprlya.fsf@alter.siamese.dyndns.org>
In-Reply-To
<CACsJy8AfaETfikvB+zON4iiX2HBehJg-xX2GGmKYHueX8rv3qw@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
Show 16 quoted lines
> On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey <drafnel@gmail.com> wrote:
>
>> I've integrated Duy's series with a few minor tweaks.  I added a couple
>> of additional tests to t4014 and corrected one of the tests which had
>> incorrect behavior.  I think his sign-off's should still be valid, so I
>> kept them in.  Sorry that I've been slow, and now the two of us are stepping
>> on each other's toes, but Duy please take a look and let me know if there's
>> anything you dislike.
>
> I'm still not sure whether format-patch should follow cherry-pick's
> rule in appending sob. If it does, it probably makes more sense to fix
> the sequencer.c code then delete log-tree.c (not fixes on log-tree.c
> then delete it). I see that your changes pass all the new tests I
> added in format-patch so sequencer.c is probably good enough,
> log-tree.c changes are probably not needed. Feel free take over the
> series :-)
After reading the series over, I agree with the above.

Patch #9 that fixes the copy in log-tree.c only to discard it in patch #11 does not seem to be the best organization of the series. Instead, perhaps we can salvage the tests in patch #9 (but mark failing ones as expecting failure) without updating the one in log-tree.c, adjust prototype in patch #10 (still broken in log-tree.c) to avoid having to make changes to the callers in patch #11, and then conclude the series with #11?

Other than the code in patches #06 and #07 that I already commented on, i.e. assignments in if () condition that make it harder to follow the logic, I did not find anything majorly objectionable in the series.

Thanks for a pleasant read.
Previous: Nguyen Thai Ngoc DuyNext: Junio C Hamano
Message 17 of 18 in “alternative unify appending of sob”
  1. 00/11 alternative unify appending of sobBrandon Casey, Nov 26, 2012
  2. 01/11 sequencer.c: remove broken support for rfc2822 continuation in footerBrandon Casey, Nov 26, 2012
  3. 02/11 t/test-lib-functions.sh: allow to specify the tag name to test_commitBrandon Casey, Nov 26, 2012
  4. 03/11 t/t3511: add some tests of 'cherry-pick -s' functionalityBrandon Casey, Nov 26, 2012
  5. 04/11 sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footerBrandon Casey, Nov 26, 2012
  6. Junio C HamanoNov 29, 2012
  7. Brandon CaseyNov 29, 2012
  8. 05/11 sequencer.c: always separate "(cherry picked from" from commit bodyBrandon Casey, Nov 26, 2012
  9. 06/11 sequencer.c: teach append_signoff how to detect duplicate s-o-bBrandon Casey, Nov 26, 2012
  10. Junio C HamanoNov 29, 2012
  11. 07/11 sequencer.c: teach append_signoff to avoid adding a duplicate newlineBrandon Casey, Nov 26, 2012
  12. 08/11 t4014: more tests about appending s-o-b linesBrandon Casey, Nov 26, 2012
  13. 09/11 format-patch: stricter S-o-b detectionBrandon Casey, Nov 26, 2012
  14. 10/11 format-patch: update append_signoff prototypeBrandon Casey, Nov 26, 2012
  15. 11/11 Unify appending signoff in format-patch, commit and sequencerBrandon Casey, Nov 26, 2012
  16. Nguyen Thai Ngoc DuyNov 26, 2012
  17. Junio C HamanoNov 29, 2012
  18. Junio C HamanoNov 26, 2012

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.