launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/rename-created-job into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #624009 Email should not be sent for new WIP merge proposals
  https://bugs.launchpad.net/bugs/624009


Incremental change to rename the jobs to reflect what they are really doing.

Nothing exciting here.
-- 
https://code.launchpad.net/~thumper/launchpad/rename-created-job/+merge/38681
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/rename-created-job into lp:launchpad/devel.

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-10-06 11:46:51 +0000
+++ lib/lp/code/configure.zcml	2010-10-18 02:38:47 +0000
@@ -289,14 +289,14 @@
     <allow interface="lp.code.interfaces.branchmergeproposal.ICreateMergeProposalJobSource"/>
   </securedutility>
 
-  <class class="lp.code.model.branchmergeproposaljob.MergeProposalCreatedJob">
+  <class class="lp.code.model.branchmergeproposaljob.MergeProposalNeedsReviewEmailJob">
     <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob"/>
-    <allow interface="lp.code.interfaces.branchmergeproposal.IMergeProposalCreatedJob"/>
+    <allow interface="lp.code.interfaces.branchmergeproposal.IMergeProposalNeedsReviewEmailJob"/>
   </class>
   <securedutility
-      component="lp.code.model.branchmergeproposaljob.MergeProposalCreatedJob"
-      provides="lp.code.interfaces.branchmergeproposal.IMergeProposalCreatedJobSource">
-    <allow interface="lp.code.interfaces.branchmergeproposal.IMergeProposalCreatedJobSource"/>
+      component="lp.code.model.branchmergeproposaljob.MergeProposalNeedsReviewEmailJob"
+      provides="lp.code.interfaces.branchmergeproposal.IMergeProposalNeedsReviewEmailJobSource">
+    <allow interface="lp.code.interfaces.branchmergeproposal.IMergeProposalNeedsReviewEmailJobSource"/>
   </securedutility>
 
   <securedutility

=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py	2010-10-07 00:51:29 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py	2010-10-18 02:38:47 +0000
@@ -17,8 +17,8 @@
     'ICodeReviewCommentEmailJobSource',
     'ICreateMergeProposalJob',
     'ICreateMergeProposalJobSource',
-    'IMergeProposalCreatedJob',
-    'IMergeProposalCreatedJobSource',
+    'IMergeProposalNeedsReviewEmailJob',
+    'IMergeProposalNeedsReviewEmailJobSource',
     'IMergeProposalUpdatedEmailJob',
     'IMergeProposalUpdatedEmailJobSource',
     'IReviewRequestedEmailJob',
@@ -633,15 +633,15 @@
         """Return a CreateMergeProposalJob for this message."""
 
 
-class IMergeProposalCreatedJob(IRunnableJob):
-    """Interface for review diffs."""
-
-
-class IMergeProposalCreatedJobSource(Interface):
-    """Interface for acquiring MergeProposalCreatedJobs."""
+class IMergeProposalNeedsReviewEmailJob(IRunnableJob):
+    """Email about a merge proposal needing a review.."""
+
+
+class IMergeProposalNeedsReviewEmailJobSource(Interface):
+    """Interface for acquiring MergeProposalNeedsReviewEmailJobs."""
 
     def create(bmp):
-        """Create a MergeProposalCreatedJob for the specified Job."""
+        """Create a needs review email job for the specified proposal."""
 
 
 class IUpdatePreviewDiffJob(IRunnableJob):

=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2010-10-18 02:38:47 +0000
@@ -64,7 +64,7 @@
     BranchMergeProposalJob,
     BranchMergeProposalJobType,
     CreateMergeProposalJob,
-    MergeProposalCreatedJob,
+    MergeProposalNeedsReviewEmailJob,
     )
 from lp.code.model.diff import PreviewDiff
 from lp.codehosting.vfs import get_lp_server
@@ -567,7 +567,7 @@
         messages = pop_notifications()
         self.assertEqual(0, len(messages))
         # Only a job created.
-        runner = JobRunner.fromReady(MergeProposalCreatedJob)
+        runner = JobRunner.fromReady(MergeProposalNeedsReviewEmailJob)
         self.assertEqual(1, len(list(runner.jobs)))
         transaction.commit()
 

=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py	2010-10-18 02:38:47 +0000
@@ -22,7 +22,7 @@
     'BranchMergeProposalJobType',
     'CodeReviewCommentEmailJob',
     'CreateMergeProposalJob',
-    'MergeProposalCreatedJob',
+    'MergeProposalNeedsReviewEmailJob',
     'MergeProposalUpdatedEmailJob',
     'ReviewRequestedEmailJob',
     'UpdatePreviewDiffJob',
@@ -86,8 +86,8 @@
     ICodeReviewCommentEmailJobSource,
     ICreateMergeProposalJob,
     ICreateMergeProposalJobSource,
-    IMergeProposalCreatedJob,
-    IMergeProposalCreatedJobSource,
+    IMergeProposalNeedsReviewEmailJob,
+    IMergeProposalNeedsReviewEmailJobSource,
     IMergeProposalUpdatedEmailJob,
     IMergeProposalUpdatedEmailJobSource,
     IReviewRequestedEmailJob,
@@ -114,11 +114,10 @@
 class BranchMergeProposalJobType(DBEnumeratedType):
     """Values that ICodeImportJob.state can take."""
 
-    MERGE_PROPOSAL_CREATED = DBItem(0, """
-        Merge proposal created
+    MERGE_PROPOSAL_NEEDS_REVIEW = DBItem(0, """
+        Merge proposal needs review
 
-        This job generates the review diff for a BranchMergeProposal if
-        needed, then sends mail to all interested parties.
+        This job sends mail to all interested parties about the proposal.
         """)
 
     UPDATE_PREVIEW_DIFF = DBItem(1, """
@@ -296,17 +295,17 @@
         return vars
 
 
-class MergeProposalCreatedJob(BranchMergeProposalJobDerived):
-    """See `IMergeProposalCreatedJob`."""
-
-    implements(IMergeProposalCreatedJob)
-
-    classProvides(IMergeProposalCreatedJobSource)
-
-    class_job_type = BranchMergeProposalJobType.MERGE_PROPOSAL_CREATED
+class MergeProposalNeedsReviewEmailJob(BranchMergeProposalJobDerived):
+    """See `IMergeProposalNeedsReviewEmailJob`."""
+
+    implements(IMergeProposalNeedsReviewEmailJob)
+
+    classProvides(IMergeProposalNeedsReviewEmailJobSource)
+
+    class_job_type = BranchMergeProposalJobType.MERGE_PROPOSAL_NEEDS_REVIEW
 
     def run(self):
-        """See `IMergeProposalCreatedJob`."""
+        """See `IMergeProposalNeedsReviewEmailJob`."""
         mailer = BMPMailer.forCreation(
             self.branch_merge_proposal, self.branch_merge_proposal.registrant)
         mailer.sendAll()
@@ -646,8 +645,8 @@
     """Construct a derived merge proposal job for a BranchMergeProposalJob."""
 
     job_classes = {
-        BranchMergeProposalJobType.MERGE_PROPOSAL_CREATED:
-            MergeProposalCreatedJob,
+        BranchMergeProposalJobType.MERGE_PROPOSAL_NEEDS_REVIEW:
+            MergeProposalNeedsReviewEmailJob,
         BranchMergeProposalJobType.UPDATE_PREVIEW_DIFF:
             UpdatePreviewDiffJob,
         BranchMergeProposalJobType.CODE_REVIEW_COMMENT_EMAIL:

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2010-10-18 02:38:47 +0000
@@ -69,7 +69,7 @@
 from lp.code.model.branchmergeproposaljob import (
     BranchMergeProposalJob,
     CreateMergeProposalJob,
-    MergeProposalCreatedJob,
+    MergeProposalNeedsReviewEmailJob,
     UpdatePreviewDiffJob,
     )
 from lp.code.tests.helpers import add_revision_to_branch
@@ -1255,7 +1255,7 @@
     def test_deleteProposal_deletes_job(self):
         """Deleting a branch merge proposal deletes all related jobs."""
         proposal = self.factory.makeBranchMergeProposal()
-        job = MergeProposalCreatedJob.create(proposal)
+        job = MergeProposalNeedsReviewEmailJob.create(proposal)
         job.context.sync()
         job_id = job.context.id
         login_person(proposal.registrant)

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposaljobs.py'
--- lib/lp/code/model/tests/test_branchmergeproposaljobs.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposaljobs.py	2010-10-18 02:38:47 +0000
@@ -33,7 +33,7 @@
     IBranchMergeProposalJobSource,
     ICodeReviewCommentEmailJob,
     ICodeReviewCommentEmailJobSource,
-    IMergeProposalCreatedJob,
+    IMergeProposalNeedsReviewEmailJob,
     IMergeProposalUpdatedEmailJob,
     IMergeProposalUpdatedEmailJobSource,
     IReviewRequestedEmailJob,
@@ -46,7 +46,7 @@
     BranchMergeProposalJobDerived,
     BranchMergeProposalJobType,
     CodeReviewCommentEmailJob,
-    MergeProposalCreatedJob,
+    MergeProposalNeedsReviewEmailJob,
     MergeProposalUpdatedEmailJob,
     ReviewRequestedEmailJob,
     UpdatePreviewDiffJob,
@@ -68,7 +68,7 @@
         """BranchMergeProposalJob implements expected interfaces."""
         bmp = self.factory.makeBranchMergeProposal()
         job = BranchMergeProposalJob(
-            bmp, BranchMergeProposalJobType.MERGE_PROPOSAL_CREATED, {})
+            bmp, BranchMergeProposalJobType.MERGE_PROPOSAL_NEEDS_REVIEW, {})
         job.sync()
         verifyObject(IBranchMergeProposalJob, job)
 
@@ -87,30 +87,30 @@
         SQLObjectNotFound is raised.
         """
         bmp = self.factory.makeBranchMergeProposal()
-        job = MergeProposalCreatedJob.create(bmp)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
         transaction.commit()
         self.assertRaises(
             AttributeError, BranchMergeProposalJobDerived.get, job.id)
         self.assertRaises(SQLObjectNotFound, UpdatePreviewDiffJob.get, job.id)
         self.assertRaises(
-            SQLObjectNotFound, MergeProposalCreatedJob.get, job.id + 1)
-        self.assertEqual(job, MergeProposalCreatedJob.get(job.id))
-
-
-class TestMergeProposalCreatedJob(TestCaseWithFactory):
+            SQLObjectNotFound, MergeProposalNeedsReviewEmailJob.get, job.id + 1)
+        self.assertEqual(job, MergeProposalNeedsReviewEmailJob.get(job.id))
+
+
+class TestMergeProposalNeedsReviewEmailJob(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer
 
     def test_providesInterface(self):
-        """MergeProposalCreatedJob provides the expected interfaces."""
+        """MergeProposalNeedsReviewEmailJob provides the expected interfaces."""
         bmp = self.factory.makeBranchMergeProposal()
-        job = MergeProposalCreatedJob.create(bmp)
-        verifyObject(IMergeProposalCreatedJob, job)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
+        verifyObject(IMergeProposalNeedsReviewEmailJob, job)
         verifyObject(IBranchMergeProposalJob, job)
 
     def test_getOperationDescription(self):
         bmp = self.factory.makeBranchMergeProposal()
-        job = MergeProposalCreatedJob.create(bmp)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
         self.assertTrue(
             job.getOperationDescription().startswith(
                 'notifying people about the proposal to merge'))
@@ -135,7 +135,7 @@
         """MergeProposalCreationJob.run sends an email."""
         self.useBzrBranches(direct_database=True)
         bmp = self.createProposalWithEmptyBranches()
-        job = MergeProposalCreatedJob.create(bmp)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
         self.assertEqual([], pop_notifications())
         job.run()
         self.assertEqual(2, len(pop_notifications()))
@@ -143,7 +143,7 @@
     def test_getOopsMailController(self):
         """The registrant is notified about merge proposal creation issues."""
         bmp = self.factory.makeBranchMergeProposal()
-        job = MergeProposalCreatedJob.create(bmp)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
         ctrl = job.getOopsMailController('1234')
         self.assertEqual([bmp.registrant.preferredemail.email], ctrl.to_addrs)
         message = (
@@ -163,7 +163,7 @@
             tree.commit('Initial commit')
         self.createBzrBranch(bmp.source_branch, tree.branch)
         self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)
-        job = MergeProposalCreatedJob.create(bmp)
+        job = MergeProposalNeedsReviewEmailJob.create(bmp)
         transaction.commit()
         self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
         job.run()
@@ -178,7 +178,7 @@
         verifyObject(IUpdatePreviewDiffJobSource, UpdatePreviewDiffJob)
 
     def test_providesInterface(self):
-        """MergeProposalCreatedJob provides the expected interfaces."""
+        """MergeProposalNeedsReviewEmailJob provides the expected interfaces."""
         bmp = self.factory.makeBranchMergeProposal()
         job = UpdatePreviewDiffJob.create(bmp)
         verifyObject(IUpdatePreviewDiffJob, job)
@@ -343,7 +343,7 @@
         update_diff.complete()
         [job] = self.job_source.iterReady()
         self.assertEqual(job.branch_merge_proposal, bmp)
-        self.assertIsInstance(job, MergeProposalCreatedJob)
+        self.assertIsInstance(job, MergeProposalNeedsReviewEmailJob)
 
     def completePendingJobs(self):
         # Mark all current pending jobs as complete
@@ -425,7 +425,7 @@
         verifyObject(IReviewRequestedEmailJobSource, ReviewRequestedEmailJob)
 
     def test_providesInterface(self):
-        """MergeProposalCreatedJob provides the expected interfaces."""
+        """ReviewRequestedEmailJob provides the expected interfaces."""
         request = self.factory.makeCodeReviewVoteReference()
         job = ReviewRequestedEmailJob.create(request)
         verifyObject(IReviewRequestedEmailJob, job)

=== modified file 'lib/lp/code/subscribers/branchmergeproposal.py'
--- lib/lp/code/subscribers/branchmergeproposal.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/subscribers/branchmergeproposal.py	2010-10-18 02:38:47 +0000
@@ -11,7 +11,7 @@
 
 from lp.code.adapters.branch import BranchMergeProposalDelta
 from lp.code.interfaces.branchmergeproposal import (
-    IMergeProposalCreatedJobSource,
+    IMergeProposalNeedsReviewEmailJobSource,
     IMergeProposalUpdatedEmailJobSource,
     IReviewRequestedEmailJobSource,
     IUpdatePreviewDiffJobSource,
@@ -27,7 +27,8 @@
     Also create a job to email the subscribers about the new proposal.
     """
     getUtility(IUpdatePreviewDiffJobSource).create(merge_proposal)
-    getUtility(IMergeProposalCreatedJobSource).create(merge_proposal)
+    getUtility(IMergeProposalNeedsReviewEmailJobSource).create(
+        merge_proposal)
 
 
 def merge_proposal_modified(merge_proposal, event):