On Tue, Dec 30, 2014 at 3:33 AM, Stefan Beller <sbel...@google.com> wrote: > On Mon, Dec 29, 2014 at 11:08 PM, Eric Sunshine <sunsh...@sunshineco.com> > wrote: >> On Mon, Dec 29, 2014 at 9:36 PM, Stefan Beller <sbel...@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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
