launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/code/javascript/productseries-setbranch.js'
> --- lib/lp/code/javascript/productseries-setbranch.js	2015-06-25 07:39:40 +0000
> +++ lib/lp/code/javascript/productseries-setbranch.js	2016-10-17 08:19:29 +0000
> @@ -113,6 +112,27 @@
>          }
>      };
>  
> +    module.onclick_git_repository_type = function(e) {
> +        /* Which Git repository type radio button was selected? */
> +        var types = document.getElementsByName('field.git_repository_type');
> +        var type = 'None';
> +        var i;
> +        for (i = 0; i < types.length; i++) {
> +            if (types[i].checked) {
> +                type = types[i].value;
> +                break;
> +            }
> +        }

var type = Y.one('input[name="field.git_repository_type"]:checked').get('value');

> +        // Linked
> +        module.set_enabled(
> +            'field.git_repository_location', type === 'link-lp');
> +        module.set_enabled('field.git_repository_name', type !== 'link-lp');
> +        module.set_enabled('field.git_repository_owner', type !== 'link-lp');
> +        // Import
> +        var is_external = (type === 'import-external');
> +        module.set_enabled('field.git_repository_url', is_external);
> +    };
> +
>      module.setup = function() {
>          Y.all('input[name="field.rcs_type"]').on(
>              'click', module.onclick_rcs_type);
> 
> === modified file 'lib/lp/registry/browser/product.py'
> --- lib/lp/registry/browser/product.py	2016-10-17 08:19:29 +0000
> +++ lib/lp/registry/browser/product.py	2016-10-17 08:19:29 +0000
> @@ -1658,18 +1664,26 @@
>          return BatchNavigator(decorated_result, self.request)
>  
>  
> -LINK_LP_BZR = 'link-lp-bzr'
> +LINK_LP = 'link-lp'
>  IMPORT_EXTERNAL = 'import-external'
>  
>  
>  BRANCH_TYPE_VOCABULARY = SimpleVocabulary((
> -    SimpleTerm(LINK_LP_BZR, LINK_LP_BZR,
> +    SimpleTerm(LINK_LP, LINK_LP,
>                 _("Link to a Bazaar branch already on Launchpad")),
>      SimpleTerm(IMPORT_EXTERNAL, IMPORT_EXTERNAL,
>                 _("Import a branch hosted somewhere else")),
>      ))
>  
>  
> +GIT_REPOSITORY_TYPE_VOCABULARY = SimpleVocabulary((
> +    SimpleTerm(LINK_LP, LINK_LP,
> +               _("Link to a Git repository already on Launchpad")),
> +    SimpleTerm(IMPORT_EXTERNAL, IMPORT_EXTERNAL,
> +               _("Import a repository hosted somewhere else")),

Worth saying "Git repository" for now, since unlike the Bazaar equivalent the VCS must match?

> +    ))
> +
> +
>  class SetBranchForm(Interface):
>      """The fields presented on the form for setting a branch."""
>  
> @@ -1706,23 +1720,37 @@
>          IBranch['owner'], __name__='branch_owner', title=_('Branch owner'),
>          description=_(''), required=True)
>  
> -
> -def create_git_fields():
> -    return form.Fields(
> -        Choice(__name__='default_vcs',
> -               title=_("Project VCS"),
> -               required=True, vocabulary=VCSType,
> -               description=_("The version control system for "
> -                             "this project.")),
> -        Choice(__name__='git_repository_location',
> -               title=_('Git repository'),
> -               required=False,
> -               vocabulary='GitRepositoryRestrictedOnProduct',
> -               description=_(
> -                   "The Git repository for this project in Launchpad, "
> -                   "if one exists, in the form: "
> -                   "~user/project-name/+git/repo-name"))
> -    )
> +    default_vcs = Choice(
> +        title=_("Project VCS"), required=True, vocabulary=VCSType,
> +        description=_("The version control system for this project."))
> +
> +    git_repository_location = Choice(
> +        title=_('Git repository'), required=False,
> +        vocabulary='GitRepositoryRestrictedOnProduct',
> +        description=_(
> +            "The Git repository for this project in Launchpad, "
> +            "if one exists, in the form: "
> +            "~user/project-name/+git/repo-name"))
> +
> +    git_repository_type = Choice(
> +        title=_('Import type'), required=True,
> +        vocabulary=GIT_REPOSITORY_TYPE_VOCABULARY,
> +        description=_('The type of import'))

Not the type of the import, but rather whether the repository is imported?

> +
> +    git_repository_name = copy_field(
> +        IGitRepository['name'], __name__='git_repository_name',
> +        title=_('Git repository name'), description=_(''), required=True)
> +
> +    git_repository_owner = copy_field(
> +        IGitRepository['owner'], __name__='git_repository_owner',
> +        title=_('Git repository owner'), description=_(''), required=True)
> +
> +    git_repository_url = URIField(
> +        title=_('Git repository URL'), required=True,
> +        description=_('The URL of the Git repository.'),
> +        allowed_schemes=["git", "http", "https"],
> +        allow_userinfo=True, allow_port=True, allow_query=False,
> +        allow_fragment=False, trailing_slash=False)
>  
>  
>  class ProductSetBranchView(ReturnToReferrerMixin, LaunchpadFormView,


-- 
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-configure-code/+merge/308577
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.