James Westby wrote: […] > bzrlib.plugins.builddeb.tests.test_merge_changelog.TestMergeChangelog.test_not_valid_changelog > > This is testing what happens with an invalid changelog. I'm not sure > what the effect of returning "not_applicable" versus "success" with the > invalid text would be. I don't think it's a particularly important case > though.
“not_applicable” gives other merge hooks, including the default one (which is always applicable, but might conflict), a chance to try the merge. The other outcomes (“success”, “conflicted” and “deleted”) are final result for the merge. So that test is saying “if it's not a valid changelog, give up and let the default logic (or other plugins) merge it.” I agree that that doesn't seem particularly important. The main thing I think is that invalid changelogs shouldn't cause tracebacks or other completely unreasonable output (and *perhaps* ideally also produce a gentle warning). If so, “success” vs. “not_applicable” are both fine, assuming the apparent success isn't actually total garbage. > bzrlib.plugins.builddeb.tests.test_merge_changelog.TestMergeChangelog.test_3way_conflicted > > Getting success back in this case seems wrong. Hmm, I can see a good argument for success here, actually. This looks like a successful three-way merge: > The text it returns is: > > psuedo-prog (1.1.1-2) unstable; urgency=low > > * New upstream release. > * Yet another content for 1.1.1-2 > * But more is better > > -- Joe Foo <[email protected]> Thu, 28 Jan 2010 10:45:44 +0000 > > and I'm not sure that's correct as it's discarding a line. > > If it's the desired behaviour of dpkg-mergechangelog then it may be what > we want, but it seems odd to me. BASE → OTHER deletes the “Awesome bug fixes.” line, and adds the “Yet another…” line. BASE → THIS just adds “But more is better”. So that output looks like a sensible and expected combination of those changes: it's what I'd do if I had to merge those by hand. But as a VCS developer perhaps my sense of what's expected is skewed ;) Given that one side does discard a line, do you still find it odd that the merge also discards that line? -Andrew. -- ubuntu-distributed-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
