Code review comment for lp:~al-maisan/bzr-builddeb/merge-package

Revision history for this message
James Westby (james-w) wrote :

Excerpts from Muharem Hrnjadovic's message of Fri Aug 21 12:51:08 UTC 2009:
> > This is the big question I have left though. How does the user continue
> > after this? There should be a clear way to continue once they have committed,
> > and the exception should tell them how to proceed.
>
> How about this?
>
> {{{
> The "merge-package" command has detected diverged upstream
> branches for the merge source and target. A shared upstream
> revision was constructed to remedy the problem.
>
> However, merging the shared upstream revision into the merge
> target resulted in conflicts.
>
> Please proceed as follows:
>
> 1 - Resolve the current merge conflicts in the merge target
> directory and commit the changes.
> 2 - Perform a plain "bzr merge <source-packaging-branch>"
> command, resolve any ensuing packaging branch conflicts
> and commit once satisfied with the changes.
> }}}

For starters this is bit verbose, we can't really format it like that
in an error message, so it needs to be a couple of sentences.

I don't think they need to know that a new "shared upstream" was
created, you can just say that there are conflicts but they are
not finished.

Should we direct them to "bzr merge-package" again? It becomes
equivalent, but I think it's better to not confuse them with
when they can use "bzr merge" and when they can't.

You don't need to tell them what to do after they've run that again,
it can just tell them when it is done.

Would it be worth saying that if they don't like what they see
they can just "bzr revert" to get back to where they started?

Looking at the incremental diff the major issue I see remaining
is that you lock and unlock the tree, only to lock it again.
Lock it once, and hold that lock until you don't reference the
tree or anything that makes use of it any more.

Writing this mail also interested me in whether you ever get any
conflicts in the case when merging the faked upstream merge
in to the packaging branch in the case where your upstream
version was newer than the other upstream version.

Thanks,

James

« Back to merge proposal