launchpad-reviewers team mailing list archive

[Colin Watson <mp+410373@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
> index 363356d..ffd53c9 100644
> --- a/lib/lp/code/interfaces/gitrepository.py
> +++ b/lib/lp/code/interfaces/gitrepository.py
> @@ -749,6 +749,33 @@ class IGitRepositoryModerateAttributes(Interface):
>          description=_("A short description of this repository.")))
>  
>  
> +class IGitRepositoryBuildStatus(Interface):

Does this make sense as a separate interface, rather than just putting the new method in `IGitRepositoryEdit`?  (It would need to be a separate interface if it needs to have different `zope.security`-level permissions, leaving aside the access token arrangements.)

> +
> +    @operation_parameters(
> +        name=TextLine(title=_("The name of the status report.")),
> +        status=List(
> +            title=_("A list of report statuses to filter by."),
> +            value_type=Choice(vocabulary=RevisionStatus)),
> +        description=TextLine(title=_("The description of the status report.")),
> +        commit_sha1=TextLine(title=_("The commit sha1 of the status report.")),
> +        result=List(
> +            title=_("A list of report result statuses to filter by."),
> +            value_type=Choice(vocabulary=RevisionStatusResult)))
> +    @scoped('repository:build_status')

Could you use `@scoped(AccessTokenScope.REPOSITORY_BUILD_STATUS.title)` here?  If we consistently use the enum, then it's harder to make typos.

> +    @call_with(user=REQUEST_USER)
> +    @export_write_operation()
> +    @operation_for_version("devel")
> +    def newRevisionStatusReport(name, status, description, commit_sha1, result, user):
> +        """Create a New Status Report and return its ID.
> +
> +        :param name: The name of the new report.
> +        :param status: The `RevisionStatus` of the new report.
> +        :param description: The description of the new report.
> +        :param commit_sha1: The commit sha1 for the report.
> +        :param result: The result of the new report.
> +        """
> +
> +
>  class IGitRepositoryModerate(Interface):
>      """IGitRepository methods that can be called by more than one community."""
>  
> diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
> index 99d451b..63ec4e8 100644
> --- a/lib/lp/code/model/gitrepository.py
> +++ b/lib/lp/code/model/gitrepository.py
> @@ -501,6 +503,11 @@ class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin,
>      def collectGarbage(self):
>          getUtility(IGitHostingClient).collectGarbage(self.getInternalPath())
>  
> +    @scoped('repository:build_status')

Only the interface needs the `@scoped` decorator.

> +    def newRevisionStatusReport(self, name, status, description, commit_sha1, result, user):
> +        """See `IGitRepositoryBuildStatus`."""
> +        print('test')
> +
>      @property
>      def namespace(self):
>          """See `IGitRepository`."""


-- 
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/410373
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:revision-status-submission-api into launchpad:master.