Code review comment for lp:~agrimm/bzr-fastimport/baseline-commit

Revision history for this message
Andy Grimm (agrimm) wrote :

> >> + assert(len(revobj.parent_ids))
> >> + previd = revobj.parent_ids[0]
> >> It seems a bit odd that the code uses the parent of the revision that was
> >> specified, rather than that revision itself.
> > I wrestled with that a little. If the baseline argument were to take a
> single revision as an argument, I would agree that that is what should be
> used. Instead, I required the user to specify a revisionspec, which indicates
> the commits which they'd like to preserve. If I did not use the parent
> revision here, then they'd lose the first diff they wanted to be able to see.
> That might be confusing, though, and I'm not set on this behavior.
> >
> > Do you think that it would be better to have the baseline option take a
> revisionspec directly, rather than requiring -r along with this option? (If I
> continue to require -r, I guess I need to writing some code to enforce that
> requirement.)
> -r specifies a revision - not a delta between revisions, so I do think
> --baseline should only include the revision that was specified and not
> its left hand side parent. For deltas (changes) we usually use the -c
> option.

Oh, I see your point here. when someone gives the command "bzr diff -r 4..6" for example, 4 is the baseline, and the diff shown is the delta between 4 and 6, so it makes sense to make the baseline the state at the first revision given. Thanks, I'll make that change.

« Back to merge proposal