launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Needs Fixing



Diff comments:

> === modified file 'lib/lp/code/browser/gitlisting.py'
> --- lib/lp/code/browser/gitlisting.py	2015-09-21 09:59:13 +0000
> +++ lib/lp/code/browser/gitlisting.py	2018-08-29 13:39:07 +0000
> @@ -104,6 +104,10 @@
>      def repos(self):
>          return GitRepositoryBatchNavigator(self, self.repo_collection)
>  
> +    @property
> +    def show_junk_directions(self):

We're mostly stuck with "junk" for Bazaar, but a lot of people don't like it for various reasons so we avoid the term for Git.  Can you replace "junk" with "personal" everywhere in this MP?

> +        return self.user == self.context
> +
>  
>  class TargetGitListingView(BaseGitListingView):
>  
> 
> === modified file 'lib/lp/code/templates/gitlisting.pt'
> --- lib/lp/code/templates/gitlisting.pt	2018-08-22 14:04:22 +0000
> +++ lib/lp/code/templates/gitlisting.pt	2018-08-29 13:39:07 +0000
> @@ -55,6 +55,14 @@
>      <span class="see-all" tal:condition="view/show_bzr_link">
>        <a tal:attributes="href context/fmt:url:code/+branches">View Bazaar branches</a>
>      </span>
> +
> +    <p id="junk-branch-directions" tal:condition="view/show_junk_directions" style="max-width: 100%;">

Rather than an inline style attribute, could you put this in lib/canonical/launchpad/icing/style.css instead, using something like "p#personal-git-directions"?

> +      You can push (upload) personal branches (those not related to a project)
> +      with the following command:

It's technically true that these directions push a single branch.  On the other hand, the instructions here are really just for completely new repositories; once the repository exists there'll be more specific instructions on the repository page.

Perhaps something like "To create new personal repositories (...), you can push a local repository with the following command:"?  This might need a bit more wordsmithing.

> +      <br />
> +      <tt class="command" >git push --set-upstream git+ssh://<tal:name replace="view/user/name"/>@git.launchpad.net/~<tal:name replace="view/user/name"/>/+git/<em>REPOSITORY_NAME</em> master</tt>

I think we should aim for something like:

  git remote add origin git+ssh://NAME@xxxxxxxxxxxxxxxxx/~NAME/+GIT/REPOSITORY_NAME
  git push --set-upstream origin master

Although that's two commands rather than one, it makes it easier for people who need to push another branch to the same repository later, and we should nudge them that way rather than possibly have them think they need to create multiple repositories for the same codebase.

> +    </p>
> +
>      <tal:default-repository
>          condition="view/default_git_repository"
>          define="repository view/default_git_repository">


-- 
https://code.launchpad.net/~twom/launchpad/git-instructions-for-people/+merge/353971
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.