launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bmp-tidying into lp:launchpad.

Commit message:
Various small bits of tidying up of merge proposals in advance of extending them to support Git.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bmp-tidying/+merge/256952

Various small bits of tidying up of merge proposals in advance of extending them to support Git.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bmp-tidying into lp:launchpad.

=== modified file 'lib/lp/app/widgets/tests/test_suggestion.py'
--- lib/lp/app/widgets/tests/test_suggestion.py	2012-12-11 05:41:50 +0000
+++ lib/lp/app/widgets/tests/test_suggestion.py	2015-04-21 15:01:44 +0000
@@ -153,7 +153,7 @@
     layer = DatabaseFunctionalLayer
 
     def makeBranchAndOldMergeProposal(self, timedelta):
-        """Make an old  merge proposal and a branch with the same target."""
+        """Make an old merge proposal and a branch with the same target."""
         bmp = self.factory.makeBranchMergeProposal(
             date_created=datetime.now(utc) - timedelta)
         login_person(bmp.registrant)

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2015-04-19 12:56:32 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2015-04-21 15:01:44 +0000
@@ -100,7 +100,6 @@
     )
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
-from lp.services.fields import Whiteboard
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.messages.interfaces.message import IMessageSet
 from lp.services.propertycache import (
@@ -132,7 +131,7 @@
         # Don't show the proposal if the user can't see it.
         if not check_permission('launchpad.View', proposal):
             continue
-        # Only show the must recent proposal for any given target.
+        # Only show the most recent proposal for any given target.
         date_created = proposal.date_created
         target_id = proposal.target_branch.id
 
@@ -938,19 +937,6 @@
                           "mark as 'Needs review'.")
 
 
-class ReviewForm(Interface):
-    """A simple interface to define the revision number field."""
-
-    revision_number = Int(
-        title=_("Reviewed Revision"), required=True,
-        description=_("The revision number on the source branch which "
-                      "has been reviewed."))
-
-    whiteboard = Whiteboard(
-        title=_('Whiteboard'), required=False,
-        description=_('Notes about the merge.'))
-
-
 class MergeProposalEditView(LaunchpadEditFormView,
                             BranchMergeProposalRevisionIdMixin):
     """A base class for merge proposal edit views."""
@@ -961,48 +947,6 @@
         self.cancel_url = self.next_url
         super(MergeProposalEditView, self).initialize()
 
-    def _getRevisionId(self, data):
-        """Translate the revision number that was entered into a revision id.
-
-        If the branch is REMOTE we won't have any scanned revisions to compare
-        against, so store the raw integer revision number as the revision id.
-        """
-        source_branch = self.context.source_branch
-        # Get the revision number out of the data.
-        if source_branch.branch_type == BranchType.REMOTE:
-            return str(data.pop('revision_number'))
-        else:
-            branch_revision = source_branch.getBranchRevision(
-                sequence=data.pop('revision_number'))
-            return branch_revision.revision.revision_id
-
-    def _validateRevisionNumber(self, data, revision_name):
-        """Check to make sure that the revision number entered is valid."""
-        rev_no = data.get('revision_number')
-        if rev_no is not None:
-            try:
-                rev_no = int(rev_no)
-            except ValueError:
-                self.setFieldError(
-                    'revision_number',
-                    'The %s revision must be a positive number.'
-                    % revision_name)
-            else:
-                if rev_no < 1:
-                    self.setFieldError(
-                        'revision_number',
-                        'The %s revision must be a positive number.'
-                        % revision_name)
-                # Accept any positive integer for a REMOTE branch.
-                source_branch = self.context.source_branch
-                if (source_branch.branch_type != BranchType.REMOTE and
-                    rev_no > source_branch.revision_count):
-                    self.setFieldError(
-                        'revision_number',
-                        'The %s revision cannot be larger than the '
-                        'tip revision of the source branch.'
-                        % revision_name)
-
 
 class ResubmitSchema(IBranchMergeProposal):
 

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2015-04-19 12:56:32 +0000
+++ lib/lp/code/interfaces/branch.py	2015-04-21 15:01:44 +0000
@@ -567,7 +567,7 @@
             value_type=Reference(Interface)))
 
     def isBranchMergeable(other_branch):
-        """Is the other branch mergeable into this branch (or vice versa)."""
+        """Is the other branch mergeable into this branch (or vice versa)?"""
 
     @export_operation_as('createMergeProposal')
     @operation_parameters(

=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py	2015-01-28 16:38:13 +0000
+++ lib/lp/code/interfaces/branchcollection.py	2015-04-21 15:01:44 +0000
@@ -207,7 +207,7 @@
     def scannedSince(epoch):
         """Restrict the collection to branches scanned since `epoch`."""
 
-    def targetedBy(person):
+    def targetedBy(person, since=None):
         """Restrict the collection to branches targeted by person.
 
         A branch is targeted by a person if that person has registered a merge

=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py	2014-11-28 22:07:05 +0000
+++ lib/lp/code/interfaces/branchtarget.py	2015-04-21 15:01:44 +0000
@@ -99,7 +99,7 @@
         "Does this target support code imports at all?")
 
     def areBranchesMergeable(other_target):
-        """Are branches from other_target mergeable into this target."""
+        """Are branches from other_target mergeable into this target?"""
 
     def __eq__(other):
         """Is this target the same as another target?"""

=== modified file 'lib/lp/code/javascript/branchmergeproposal.nominate.js'
--- lib/lp/code/javascript/branchmergeproposal.nominate.js	2013-03-20 22:32:47 +0000
+++ lib/lp/code/javascript/branchmergeproposal.nominate.js	2015-04-21 15:01:44 +0000
@@ -17,7 +17,7 @@
  * @param branches_to_check
  * @param branch_info
  */
-var _make_confirm_propmpt = function(branches_to_check, branch_info) {
+var _make_confirm_prompt = function(branches_to_check, branch_info) {
     var visible_branches = branch_info.visible_branches;
     var invisible_branches = Y.Array.filter(branches_to_check, function(i) {
         return Y.Array.indexOf(visible_branches, i) < 0;
@@ -54,8 +54,7 @@
     var visible_branches = branch_info.visible_branches;
     if (Y.Lang.isArray(visible_branches)
             && visible_branches.length !== branches_to_check.length) {
-        var yn_content = _make_confirm_propmpt(
-            branches_to_check, branch_info);
+        var yn_content = _make_confirm_prompt(branches_to_check, branch_info);
         Y.lp.app.picker.yesno_save_confirmation(
                 picker, yn_content, "Nominate", "Choose Again",
                 save_fn, cancel_fn);
@@ -120,7 +119,7 @@
  */
 var confirm_reviewer_nomination = function(branch_info) {
     var branches_to_check = branch_info.branches_to_check;
-    var yn_content = _make_confirm_propmpt(branches_to_check, branch_info);
+    var yn_content = _make_confirm_prompt(branches_to_check, branch_info);
     var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
         submit_fn: function() {
             var form = Y.one("[name='launchpadform']");

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2015-04-19 12:56:32 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2015-04-21 15:01:44 +0000
@@ -375,7 +375,7 @@
             recipients[self.registrant] = RecipientReason.forRegistrant(
                 self, branch_identity_cache=branch_identity_cache)
         # If the owner of the source branch is getting emails, override the
-        # rationale to say they are the owner of the souce branch.
+        # rationale to say they are the owner of the source branch.
         source_owner = self.source_branch.owner
         if source_owner in recipients:
             reason = RecipientReason.forSourceOwner(