launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve

The instances of +branch in blueprints are traversals under Specification, which shouldn't be changed here.  (Linking directly to Git branches there wouldn't be appropriate, but we may at some point allow them to be linked to merge proposals, much like the similar work I'm doing for bugs.)

I think the traversal tests in lp.code.browser.tests.test_branchtraversal are under Person rather than under the root, so they also shouldn't be changed.

I'm happy with most of this now, but there is one oddity.  Revision 18107 in your branch seems to have partially reverted some previous fixes you applied in response to my previous review feedback, so e.g. it still has the dubious check for '+git' as a path substring.  Was there a particular reason for that, or was it an accident?  If that was in response to some test failures (query counts maybe?), I think we need to find a different fix to those.  So I'm casting an Approve review in order not to block you, but please do fix that up before landing.
-- 
https://code.launchpad.net/~thomir/launchpad/devel-fix-git-links/+merge/297963
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.