>>>>> "jam" == John A Meinel writes: jam> Vincent Ladeuil wrote: >>>>>>> "jam" == John A Meinel writes: >> >> >> jam> Also... jam> We should *definitely* be checking: jam> tree.last_revision() == tree.branch.last_revision() jam> And possibly consider what to do in the case of bound jam> branches and heavyweight checkouts. I'm not *as* jam> concerned about the latter, but because in the former jam> case, IIRC we will actually be pushing jam> "tree.branch.last_revision()" *not* jam> tree.last_revision()... >> >> Hmm, so far --strict is defined as triggering if there are >> *uncommitted* changes, you're stretching the definition here... jam> So I can agree that it is a different bug except: jam> 1) I think 'bzr push --strict' is about 'push should jam> push the state I *think* I have'. I understood that :) jam> And uncommitted changes, (uncommitted merges), and a jam> tree last revision being out of sync with the branch's jam> revision, are all cases where the push isn't pushing jam> what the user expected. Right. You're right. I agree with the rationale, but... I don't clearly understand when a user can find itself in this situation... I can see one case for a bound branch, except that I can't commit in that situation so I have nothing to push... Or maybe you mean a commit --local ? But in that case push should already fail with "diverged branches" (strict or no strict) no ? jam> 2) I think the source of '--strict' is MySQL fallout jam> from switching from BK where a merge had autocommit so jam> "bk merge; bk push" did something, while with bzr you jam> need "bzr merge; bzr commit; bzr push". I agree here too, mysql workflow wasn't my main reason for agreeing about making --strict the default. In fact my first fix addressed mysql workflow *without* making it he default. I think the default should be strict because uneducated users will easily forget to commit before pushing, and even experienced ones may prefer, for some branches, to work in strict mode. jam> 3) The fix is fairly trivial, the bug is somewhat jam> serious, and you're already working on the code. Yes it jam> is feature creep, but it is a small amount for something jam> worthwhile (IMO). I don't have a problem with working more on that, but I just don't know how to write the tests (the what, not the how). jam> I read your links, and while there wasn't a general jam> discussion on it, Martin at least seems to agree it jam> should be the default. Sorry to have jumped to that conclusion without discussing it first then. jam> I'm just a bit extra sensitive to commandline behavioral jam> changes. Since suddenly something that used to work jam> won't anymore. Well, I thought about that and I came to the conclusion that it's pretty rare than one find itself in the situation to push while having uncommitted changes. At least to me, it sounds like advanced usage and I don't mind having to add an option in that case, even if I didn't have to do it before, especially since I can configure that behavior at any level. jam> Also, note that "UncommittedChanges" has no help text to jam> tell the user that they can use "--no-strict" in order jam> to push anyway. I don't know if that matters or not. But jam> since we are explictly changing a behavior, it is good jam> to help users find out how to get their old behavior jam> back. Hmm, good point. First they need to be directed to 'bzr status' and then to 'push --no-strict' though. I'll add something in that direction before landing then, feel free to propose a better one, there will be a follow-up submission obviously. jam> Anyway, you can probably merge this as is, though I'd jam> like it if you added a tree.last_revision() check. Then let's discuss that after I land that patch because I just don't know how to address your concern otherwise.