2009/6/9 Junio C Hamano <gitster@xxxxxxxxx>: > Santi Béjar <santi@xxxxxxxxxxx> writes: > >>> git pull --rebase tags v1.6.0 >> >> In fact: git pull --rebase remote tags v1.6.0 >> >> But this still works because oldremoteref defaults to defaults_merge. >> So the only behavior change is when a remote branch is >> rebased/retagged, and you have worst problems then. I think noone used >> the rebased functionality in this way, so I don't think it is worth to >> support it. But if someone think it is important I'll do it. > > I personally do not think supporting such a form of input is absolutely > necessary. Even though technically it might be a regression, if it is so > rare a form, we can simply say "this strange form used to work, but now it > does not; you can use this form instead to do the same thing", and move > on. OK. > > However, at least we should describe the change, both in the commit log > and documentation. Simply saying "No behaviour change" is not acceptable, > when the code clearly is doing something else. It needs to be backed by > some explanation, e.g. "Even though this returns different results from > the original, the caller behaves the same because of such and such > reasons". OK. > > What caught my attention was not the difference between the new code and > the original codepath, but your "FIXME" comment that said "Currently only > works with the default mapping". My initial reaction was "What? The new > code that introduces a function for the specific task of figuring out the > mapping does not work if the user uses a custom mapping? What kind of > improvement is that???". The original code (in git-pull.sh) behaves like this, I only made it more explicit. > > The reaction was followed by "Even if that were the case, if the original > code did not work in the case anyway, then it is not a regression. The > proposed commit log message claims that there is no behaviour change, so > that FIXME might not be so grave an offense. Is it really the case? Was > the original broken?" Yes, the original is broken. > > While trying to figure it out, I noticed that the new code does quite a > different thing (I still haven't figured out the answer to my original > question about FIXME, by the way). > > In any case, if we were changing behaviour by deprecating support for a > rarely-if-ever used syntax, it would be nice if we at least diagnosed it, > instead of failing, or worse yet, silently doing something different from > the old behaviour. I am not changing the behaviour. The old code worked exactly like the new one. But I agree that this has to be documented(/deprecated?) somewhere. The only change in behavior is when a remote tag is rebased/retagged. Santi -- 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