Rudolf Polzer <divVerent@xxxxxxxxxxxxx> writes: > On Thu, Jan 14, 2010 at 09:47:22PM -0800, Junio C Hamano wrote: >> I have a feeling that it is more appropriate to have the additional code >> in transport_push(), which gets ls-remote information, runs match_refs() >> and finally calls transport->push_refs(). I think the extra branch >> configuration would fit better inside the if block immediately after all >> that happens, i.e. >> >> if (!(flags & TRANSPORT_PUSH_DRY_RUN)) { >> struct ref *ref; >> for (ref = remote_refs; ref; ref = ref->next) >> update_tracking_ref(transport->remote, ref, verbose); >> + if (flags & TRANSPORT_PUSH_RECONFIGURE_FORK) >> + configure_forked_branch(...); >> } >> >> in transport.c > > I thought about this place when making my patch, but didn't put it there > because this function is not called in the rsync protocol (which defines > transport->push). That's not a very good reasoning. Instead of punishing well behaved transport that defines push_ref, punish _only_ the transports that does not define it (see the paragraph at the end of this message). > But well. Why bother with this, if this feature was rejected before already > anyway. Read the thread Nana quoted for you again; I nor anybody ever _rejected_ the ultimate goal, even though I said that the justifications were not sufficiently convincing for the previous implementation attempts. I think Ilari's patch is done right and can be extended by anybody who cares about rsync transport to call an extra ls-remote in the "does this one lack push_ref but know how to push" codepath. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html