[MERGE] merge and pull support merge directives
Aaron Bentley
aaron.bentley at utoronto.ca
Sun Apr 1 04:21:14 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
John Arbash Meinel wrote:
> John Arbash Meinel has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
> For -r, we could evaluate it after installing the revisions (since then
> we do have a copy of the branch).
>
> I don't really like that "get_target_revision()" has a side effect of
> installing revisions. It doesn't seem correct for a "get_*" function.
> Maybe: "install_revisions()" which is defined as returning the tip
> revision.
Okay.
> I realize it makes the review a bit more difficult, but we really should
> try to use "revision_id" instead of "rev_id". At least it is listed in
> HACKING: ``revision_id`` not ``rev_id`` or ``revid``
Sure. I was just following what the code was already doing.
> I really don't like that you have to fake the --revision argument with:
> + other_rev_id = mergeable.get_target_revision(
> + tree.branch.repository)
> + revision = [RevisionSpec.from_string(
> + 'revid:' + other_rev_id)]
>
> To me that just means the merge api is incorrect. And certainly you
> don't have to fix that here.
Yeah, I didn't like that, either. None of the merge APIs are ideal, but
_merge_helper is the only one that supports all the possible
invocations, so I stayed with that.
> Why are all these tests called "test_manual_*"
> + def test_manual_merge_user_r(self):
I intended merge directives to be used with automated systems like PQM
and Bundle Buggy. Compared to that, this is "manual". I can take it out.
> So the only thing I would block on is that the function is called
> "get_target_revision" and has the side effect of installing revisions.
> The rest is just discussion.
Okay, I can do that, the rev_id change, and the "manual" thing. I'd
like to save the _merge_helper updates for a separate change.
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGDyUp0F+nu1YWqI0RAqQwAJ9U4xTSSxkrPHqoslu1lqbnnQOivQCfUZ9X
D9Y5OvxjVXlqvKvnAx3Ga9s=
=PEds
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list