Thank you for this branch. Cleaning out the no-longer-working doctests is nice
too, though hooking them up again, insofar as they actually were documentation,
might not have been horrible. Thank you also for changing "directory" to
"repository".
I don't want to forget the branch argument: if we have time, we should add it.
Please add a low priority card to the kanban board for this task.
Using get to update an existing checkout is interesting, and I'm not sure it is
the right thing. That's the intended domain of update, which also is supposed
to shelve changes before the pull and unshelve them after the pull. I suggest
that we rip that behavior out from get. (Note that IMO we can also rip the
branch command too, if that makes anything easier; but that can wait till
another branch too. Note further that we can rip out the install command, but
that's trickier because lxcinstall defers to it ATM, so I definitely think that
ripping install out needs to be its own branch.)
I was curious about the "Repository {0} is corrupted" stuff when I saw it
too...did you check with frankban before you removed it? If not, please do. I
think either removing it or commenting on why we think it is necessary are both
good possible directions.
If there is no sandbox_dir, and only a checkout_dir, can we infer that the
checkout is actually a branch (i.e., lightweight=False)?
ISTR someone was going to check on why we have --2a as an argument. That
should be the default for all versions of bzr that we encounter. If it is not,
I think a comment is appropriate. Otherwise, let's remove it.
I skimmed the rest, and it looked good. I'm not going to approve pending
changes because I think there may need to be a discussion about one or the both
of inferring the "lightweight" value and ripping out get's "update" behavior.
If someone else wants to take over the rest of the review, that would be cool.
Thanks again,
Gary
--
https://code.launchpad.net/~benji/lpsetup/bug-1016645-add-command-line-options/+merge/112588
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~benji/lpsetup/bug-1016645-add-command-line-options into lp:lpsetup.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
