launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/code/subscribers/karma.py'
> --- lib/lp/code/subscribers/karma.py	2015-04-22 16:11:40 +0000
> +++ lib/lp/code/subscribers/karma.py	2015-10-06 15:19:46 +0000
> @@ -43,26 +45,33 @@
>  
>  
>  @block_implicit_flushes
> -def branch_merge_status_changed(proposal, event):
> -    """Assign karma to the user who approved the merge."""
> +def branch_merge_modified(proposal, event):
> +    """Assign karma to the user who approved or rejected the merge."""
> +    if event.user is None or isinstance(event.user, UnauthenticatedPrincipal):

When does an unauthenticated request modify a BMP?

> +        # Some modification events have no associated user context.  In
> +        # these cases there's no karma to assign.
> +        return
> +
>      if proposal.source_git_repository is not None:
>          target = proposal.source_git_repository.namespace
>      else:
>          target = proposal.source_branch.target
>      user = IPerson(event.user)
> +    old_status = event.object_before_modification.queue_status
> +    new_status = proposal.queue_status
>  
>      in_progress_states = (
>          BranchMergeProposalStatus.WORK_IN_PROGRESS,
>          BranchMergeProposalStatus.NEEDS_REVIEW)
>  
> -    if ((event.to_state == BranchMergeProposalStatus.CODE_APPROVED) and
> -        (event.from_state in (in_progress_states))):
> +    if ((new_status == BranchMergeProposalStatus.CODE_APPROVED) and
> +        (old_status in (in_progress_states))):
>          if user == proposal.registrant:
>              target.assignKarma(user, 'branchmergeapprovedown')
>          else:
>              target.assignKarma(user, 'branchmergeapproved')
> -    elif ((event.to_state == BranchMergeProposalStatus.REJECTED) and
> -          (event.from_state in (in_progress_states))):
> +    elif ((new_status == BranchMergeProposalStatus.REJECTED) and
> +          (old_status in (in_progress_states))):
>          if user == proposal.registrant:
>              target.assignKarma(user, 'branchmergerejectedown')
>          else:


-- 
https://code.launchpad.net/~cjwatson/launchpad/consistent-bmp-events/+merge/273325
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.