Code review comment for lp:~thumper/launchpad/branch-move-methods

Revision history for this message
Tim Penhey (thumper) wrote :

= Summary =

This branch adds setOwner and setTarget to IBranch.

IBranch.owner and IBranch.product are changed to be read only in the interface
and API.

== Implementation details ==

Both setOwner and setTarget defer to the IBranchNamespace.moveBranch method.
This method does the validation checks for both owner setting, name clashes,
and visibility policy checks.

The LaunchpadEditForm view updateContextFromData was modified to take a boolean
to determine whether or not the ObjectModifiedEvent listeners should be
notified. This is because the setOwner call needs to remove the 'owner' key
out of the dict as the updateContextFromData will fail if it is there, but we
still want to show that the change has occurred in the email.

A number of changes are just noise in the move to make owner and product read
only through the interface.

== Tests ==

TestBranchSetOwner
TestBranchSetTarget
TestBranchNamespaceMoveBranch
test_retargetBranch

== Demo and Q/A ==

Not much has chanced in the UI. If you can still change the owner of the
branch, then we have succeeded.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/webapp/launchpadform.py
  lib/lp/code/interfaces/branchtarget.py
  lib/lp/code/configure.zcml
  lib/lp/code/browser/branch.py
  lib/lp/code/interfaces/branch.py
  lib/lp/registry/doc/private-team-roles.txt
  lib/lp/code/interfaces/branchnamespace.py
  lib/lp/registry/browser/tests/private-team-creation-views.txt
  lib/lp/code/model/branchtarget.py
  lib/lp/codehosting/tests/test_acceptance.py
  lib/lp/code/model/branch.py
  lib/lp/code/browser/tests/test_branchmergeproposallisting.py
  lib/lp/code/model/tests/test_branchtarget.py
  lib/lp/code/model/tests/test_branchnamespace.py
  lib/lp/code/model/branchnamespace.py
  lib/lp/code/doc/branch.txt
  lib/lp/code/model/tests/test_revision.py
  lib/lp/code/scripts/tests/test_revisionkarma.py
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/code/model/tests/test_branch.py

== Pyflakes notices ==

lib/canonical/launchpad/webapp/launchpadform.py
    22: 'action' imported but unused
    94: redefinition of unused 'action' from line 22
    195: redefinition of unused 'action' from line 22

lib/lp/code/model/branch.py
    73: 'IPerson' imported but unused

lib/lp/code/model/tests/test_branch.py
    61: 'IProductSet' imported but unused

== Pylint notices ==

lib/canonical/launchpad/webapp/launchpadform.py
    245: [W0211, LaunchpadFormView.validate_none] Static method with 'self' as
first argument

lib/lp/code/browser/branch.py
    48: [F0401] Unable to import 'lazr.uri' (No module named uri)

lib/lp/code/model/branch.py
    186: [C0301] Line too long (79/78)
    73: [W0611] Unused import IPerson

lib/lp/code/browser/tests/test_branchmergeproposallisting.py
    16: [C0301] Line too long (79/78)

lib/lp/code/scripts/tests/test_revisionkarma.py
    52: [C0322, TestRevisionKarma.test_junkBranchMoved] Operator not preceded
by a space
    project=self.factory.makeProduct()
    ^

lib/lp/code/model/tests/test_branch.py
    61: [W0611] Unused import IProductSet

« Back to merge proposal