Code review comment for ~ilasc/launchpad:git-push-url-hint

git_ssh_url_non_owner works just fine with a single code path in my tests; you're passing target_default=False when constructing ContributorGitIdentity, so the breakage you mention doesn't happen. It does require some test changes to account for the resulting URLs being different, but IMO the changed URLs are an improvement. See this patch:

  https://paste.ubuntu.com/p/gS5YWSnDYB/

There's this new import policy warning from the test suite, which should be resolved one way or another:

There were 2 imports of names not appearing in the __all__.
You should not import ContributorGitIdentity from lp.code.interfaces.gitrepository:
    lp.code.browser.gitref
    lp.code.browser.gitrepository

There's also one further case that may have been overlooked here. Personal repositories (those handled by lp.code.model.gitnamespace.PersonalGitNamespace, of the form /~OWNER/+git/NAME) don't support merge proposals between different repositories, because there's no obvious link between them except possibly for the name. This isn't ideal, and it might be worth us revisiting it in the future, but for the meantime this is how it is. As such, it risks misleading users if, for example, I visit /~ilasc/+git/random-scripts and am told that I can push code to /~cjwatson/+git/random-scripts and propose changes from there. We should probably stick with the existing "You cannot push to this branch/repository" language in that case and not try to provide a push URL hint, even though it's unfortunate that that complicates this branch some more.

Despite my repeated "Needs fixing" votes, this is definitely improving, and I think is nearly there! Hopefully it should just be this one more round.