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

Re: [PATCHv8 1/9] receive-pack.c: add documentation for atomic push support

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Dec 30, 2014, 09:09 UTC
Message-ID
<CAPig+cSuf0yXE1BE=tCTDrcxRMEZ_pU9VU_Q9F+Jxk-xNrPdgg@mail.gmail.com>
In-Reply-To
<CAGZ79kZ_yiA7tMv49qp3OOMfOmwdaDtWs_Gt9V359xCKUE_SJg@mail.gmail.com>
On Tue, Dec 30, 2014 at 3:33 AM, Stefan Beller <sbeller@google.com> wrote:
Show 23 quoted lines
> On Mon, Dec 29, 2014 at 11:08 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Mon, Dec 29, 2014 at 9:36 PM, Stefan Beller <sbeller@google.com> wrote:
>>> Subject: receive-pack.c: add documentation for atomic push support
>> When Michael raised the issue of the server being "broken" due to
>> advertising a capability which it does not yet implement, his
>> recommendation[1] was to reorder the patches, not to split out the one
>> tiny bit (capability advertisement) from the larger change. Was there
>> an insurmountable conflict which prevented you from reordering the
>> patches? This is a genuine question since splitting off advertisement
>> -- and only advertisement -- to a patch later in the series smells
>> like a least-resistance approach, rather than a proper solution to the
>> issue Michael raised.
>
> Well there was no syntactical problem (i.e. the interactive rebase
> went flawless),
> but rather a semantic dependency. The reordered patches would not compile
> as we'd heavily depend on the use_atomic variable.
>
> Of course that could have been introduced where required, but at the
> time it did
> not look appealing to me.
>
> I'll reword the commit message header to mention the negotiation part as well.

This still smells like a least-resistance (lazy) approach. You shouldn't need the stand-alone patch enabling "atomic" advertisement just to address Michael's concern. His suggestion[1] to reorder the patches sounds sensible and correct. Toward that end, moving the declaration of 'use_atomic' to the patch where it is first needed makes sense, even if the machinery for manipulating that variable arrives with a later patch.

[1]: http://article.gmane.org/gmane.comp.version-control.git/261793
Previous: Stefan BellerNext: Stefan Beller
Message 5 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.