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

Reply via email to