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

Re: [PATCHv8 5/9] receive-pack.c: move transaction handling in a central place

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Dec 30, 2014, 20:33 UTC
Message-ID
<CAPig+cRWLKKw-AX_YqsKxLNUUCyMDmm_9AzkhUJGaYQhFmi=pQ@mail.gmail.com>
In-Reply-To
<CAGZ79kYufUcj858HGjJPTDGJgmTRv-YVR30zOtAURpP00NWEeQ@mail.gmail.com>
On Tue, Dec 30, 2014 at 1:45 PM, Stefan Beller <sbeller@google.com> wrote:
Show 16 quoted lines
> On Tue, Dec 30, 2014 at 12:36 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Mon, Dec 29, 2014 at 9:36 PM, Stefan Beller <sbeller@google.com> wrote:
>>> No functional changes intended.
>>
>> Secondary information can be demoted to the end of the commit message.
>
> I think this would help in case there is a subtle bug introduced with
> such a commit.
> If you bisect it 2 years later and then ask yourself if that subtle
> behavior was
> intentional (can easily happen if the commit message is
> short/unclear). This would then
> tell you it's definitely a bug introduced. I believe to have seen such
> a comment
> somewhere in the history, but I cannot find it again. I'll drop it
> into the notes for now.

I think you misread or misunderstood my comment, which was that secondary information should follow primary information in the commit message. I did not suggest moving it below the "---" line.

"No functional changes intended" is meaningful and belongs in the commit message, but it's not as important as the explanation and justification of the change.

Show 14 quoted lines
>>> +static void execute_commands_loop(struct command *commands,
>>> +                                 struct shallow_info *si)
>>
>> Style: Indent the wrapped line to align with the text following the
>> '(' in the first line.
>
> That's true. I have found this problem myself at another patch by
> Michael lately.
> If you apply the patch it is correctly aligned. If you view the patch
> however it is missaligned.
> Because of the leading plus sign the line in which the function
> signature starts is
> indented by one character. The other lines starting with a tab indent
> only to 8 character.
You're right. This one does align properly. Sorry for the noise.

(Of the two pointed out in patch 6/9, the first one also aligns correctly; but the second does not.)

Previous: Stefan BellerNext: Stefan Beller
Message 19 of 24 in “[PATCHv8 0/9] atomic pushes”
  1. Stefan BellerDec 30, 2014
  2. 1/9 receive-pack.c: add documentation for atomic push supportStefan Beller, Dec 30, 2014
  3. Eric SunshineDec 30, 2014
  4. Stefan BellerDec 30, 2014
  5. Eric SunshineDec 30, 2014
  6. 2/9 send-pack: rename ref_update_to_be_sent to check_to_send_updateStefan Beller, Dec 30, 2014
  7. 3/9 send-pack.c: add --atomic command line argumentStefan Beller, Dec 30, 2014
  8. 4/9 receive-pack.c: simplify execute_commandsStefan Beller, Dec 30, 2014
  9. Eric SunshineDec 30, 2014
  10. Stefan BellerDec 30, 2014
  11. Stefan BellerDec 30, 2014
  12. Eric SunshineDec 30, 2014
  13. Eric SunshineDec 30, 2014
  14. Stefan BellerDec 30, 2014
  15. Eric SunshineDec 30, 2014
  16. 5/9 receive-pack.c: move transaction handling in a central placeStefan Beller, Dec 30, 2014
  17. Eric SunshineDec 30, 2014
  18. Stefan BellerDec 30, 2014
  19. Eric SunshineDec 30, 2014
  20. 6/9 receive-pack.c: add execute_commands_atomic functionStefan Beller, Dec 30, 2014
  21. Eric SunshineDec 30, 2014
  22. 7/9 receive-pack.c: enable atomic push protocol supportStefan Beller, Dec 30, 2014
  23. 8/9 push.c: add an --atomic argumentStefan Beller, Dec 30, 2014
  24. 9/9 t5543-atomic-push.sh: add basic tests for atomic pushesStefan Beller, Dec 30, 2014

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.