Mark Phippard wrote: > Julian Foad wrote: >> By the way, my intent is to remove the '--symmetric' option and >> have the code just run inside the original 'sync' and >> 'reintegrate' code paths. There's no reason, other than for >> testing, to expose this at the UI. > > I assume the plan is that you would not need to specify --reintegrate > either?
Correct, you would no longer *need* to specify it. > Is that how you would ultimately change the test suite ... > just remove all places where we add the --reintegrate option? I > imagine the merge command still has to accept the --reintegrate option > but we would mark it as deprecated in the help text or something? > Does the code still do any of the pre-merge verifications that > --reintegrate does? Or are those checks just not needed any more? I suggest we should leave the --reintegrate option available, meaning "do stricter checks", and after doing the checks it will run the symmetric merge code. Currently the checks are "no local mods" and "no switched subtrees" and "no cherry picks"[1], in addition to the "no mixed revs unless overridden" that applies to all merges. In the test suite, at least one test verifies that the --reintegrate option does in fact do the strict WC checks it claims, and so I won't remove that option from the tests. These kind of checks can be useful for preserving the user's sanity, not just for protecting us against running code in situations we don't fully handle. We might want to consider increasing the default checks on all merges, for example to include "no switched subtrees". - Julian [1] The "no cherry picks" check -- I need to check that, as I'm not clear whether/when/how that currently applies. I expect it currently only checks one direction but should check both directions. My intention would be to apply it only when "--reintegrate" is given.