launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py	2018-10-03 00:53:55 +0000
> +++ lib/lp/code/model/gitrepository.py	2018-10-03 00:53:56 +0000
> @@ -1185,6 +1192,12 @@
>          return Store.of(self).find(
>              GitRuleGrant, GitRuleGrant.repository_id == self.id)
>  
> +    def getActivity(self):
> +        """See `IGitRepository`."""
> +        clauses = [GitActivity.repository_id == self.id]
> +        return Store.of(self).find(GitActivity, *clauses).order_by(
> +            Desc(GitActivity.date_changed), Desc(GitActivity.id))

Could/should this be a property, like grants/rules?

> +
>      def canBeDeleted(self):
>          """See `IGitRepository`."""
>          # Can't delete if the repository is associated with anything.
> 
> === modified file 'lib/lp/code/model/gitrule.py'
> --- lib/lp/code/model/gitrule.py	2018-10-03 00:53:55 +0000
> +++ lib/lp/code/model/gitrule.py	2018-10-03 00:53:56 +0000
> @@ -48,7 +51,10 @@
>      events on Git repository rules.
>      """
>      if event.edited_fields:
> -        rule.date_last_modified = UTC_NOW
> +        user = IPerson(event.user)
> +        getUtility(IGitActivitySet).logRuleChanged(
> +            event.object_before_modification, rule, user)
> +        removeSecurityProxy(rule).date_last_modified = UTC_NOW

Should this rSP be in git-permissions-model?

>  
>  
>  @implementer(IGitRule)
> @@ -126,7 +135,10 @@
>      events on Git repository grants.
>      """
>      if event.edited_fields:
> -        grant.date_last_modified = UTC_NOW
> +        user = IPerson(event.user)
> +        getUtility(IGitActivitySet).logGrantChanged(
> +            event.object_before_modification, grant, user)
> +        removeSecurityProxy(grant).date_last_modified = UTC_NOW

This one too.

>  
>  
>  @implementer(IGitRuleGrant)


-- 
https://code.launchpad.net/~cjwatson/launchpad/git-activity-model/+merge/354399
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.