launchpad-reviewers team mailing list archive

[Colin Watson <mp+428046@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-unsixify-bzr-revisions into launchpad:master.

Commit message:
Fix some errors in removing six from bzr revision handling

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/428046
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-unsixify-bzr-revisions into launchpad:master.

diff --git a/lib/lp/code/interfaces/branchjob.py b/lib/lp/code/interfaces/branchjob.py
index edc4932..30df4d2 100644
--- a/lib/lp/code/interfaces/branchjob.py
+++ b/lib/lp/code/interfaces/branchjob.py
@@ -21,6 +21,7 @@ __all__ = [
     "IRosettaUploadJobSource",
 ]
 
+from typing import Optional
 
 from zope.interface import Attribute, Interface
 from zope.schema import Bool, Bytes, Int, Object, Text, TextLine
@@ -123,10 +124,14 @@ class IRosettaUploadJob(IRunnableJob):
 
 
 class IRosettaUploadJobSource(IJobSource):
-    def create(branch, from_revision_id, force_translations_upload):
+    def create(
+        branch,
+        from_revision_id: Optional[str],
+        force_translations_upload: bool = False,
+    ):
         """Construct a new object that implements IRosettaUploadJob.
 
-        :param branch: The database branch to exract files from.
+        :param branch: The database branch to extract files from.
         :param from_revision_id: The revision id to compare against.
         :param force_translations_upload: Flag to override the settings in the
             product series and upload all translation files.
diff --git a/lib/lp/code/model/branchjob.py b/lib/lp/code/model/branchjob.py
index 09424d5..24f2793 100644
--- a/lib/lp/code/model/branchjob.py
+++ b/lib/lp/code/model/branchjob.py
@@ -18,6 +18,7 @@ import operator
 import os
 import shutil
 import tempfile
+from typing import Optional
 
 import six
 import transaction
@@ -845,14 +846,19 @@ class RosettaUploadJob(BranchJobDerived):
         return not productseries.is_empty()
 
     @classmethod
-    def create(cls, branch, from_revision_id, force_translations_upload=False):
+    def create(
+        cls,
+        branch,
+        from_revision_id: Optional[str],
+        force_translations_upload: bool = False,
+    ):
         """See `IRosettaUploadJobSource`."""
         if branch is None:
             return None
 
         if from_revision_id is None:
-            from_revision_id = NULL_REVISION
-        from_revision_id = from_revision_id.decode()
+            from_revision_id = NULL_REVISION.decode()
+        from_revision_id = from_revision_id
 
         if force_translations_upload or cls.providesTranslationFiles(branch):
             metadata = cls.getMetadata(
diff --git a/lib/lp/code/model/tests/test_branchjob.py b/lib/lp/code/model/tests/test_branchjob.py
index 24aeb5b..2b17a43 100644
--- a/lib/lp/code/model/tests/test_branchjob.py
+++ b/lib/lp/code/model/tests/test_branchjob.py
@@ -651,7 +651,7 @@ class TestRevisionsAddedJob(TestCaseWithFactory):
         job = RevisionsAddedJob.create(
             target_branch, "rev2b-id", "rev2b-id", ""
         )
-        self.assertEqual([desired_proposal], job.findRelatedBMP(["rev2a-id"]))
+        self.assertEqual([desired_proposal], job.findRelatedBMP([b"rev2a-id"]))
 
     def test_getAuthors(self):
         """Ensure getAuthors returns the authors for the revisions."""
diff --git a/lib/lp/codehosting/scanner/tests/test_bzrsync.py b/lib/lp/codehosting/scanner/tests/test_bzrsync.py
index 0b7f81c..7616e38 100644
--- a/lib/lp/codehosting/scanner/tests/test_bzrsync.py
+++ b/lib/lp/codehosting/scanner/tests/test_bzrsync.py
@@ -674,8 +674,8 @@ class TestBzrSyncRevisions(BzrSyncTestCase):
 
         # Fake revision with negative timestamp.
         fake_rev = BzrRevision(
-            revision_id="rev42",
-            parent_ids=["rev1", "rev2"],
+            revision_id=b"rev42",
+            parent_ids=[b"rev1", b"rev2"],
             committer=self.factory.getUniqueString(),
             message=self.LOG,
             timestamp=old_timestamp,
@@ -690,7 +690,7 @@ class TestBzrSyncRevisions(BzrSyncTestCase):
         # Find the revision we just synced and check that it has the correct
         # date.
         revision = getUtility(IRevisionSet).getByRevisionId(
-            fake_rev.revision_id
+            fake_rev.revision_id.decode()
         )
         self.assertEqual(old_date, revision.revision_date)