Review: Approve
Diff comments:
>
> === modified file 'lib/lp/archivepublisher/signing.py'
> --- lib/lp/archivepublisher/signing.py 2016-05-26 10:46:04 +0000
> +++ lib/lp/archivepublisher/signing.py 2016-05-26 15:57:07 +0000
> @@ -14,6 +14,7 @@
> __metaclass__ = type
>
> __all__ = [
> + "UefiUpload",
> "SigningUpload",
> ]
Please sort.
>
> @@ -61,7 +62,10 @@
> publisher configuration. In this directory, the private key is
> "uefi.key" and the certificate is "uefi.crt".
> """
> - custom_type = "signing"
> + custom_type = "raw-signing"
custom_type shouldn't have the "raw-" prefix; none of the other implementations
have that.
> +
> + def distsDirectory(self):
> + return 'signed'
A method is overkill. This could just be a simple class attribute:
"dists_directory = 'signed'"
>
> @staticmethod
> def parsePath(tarfile_path):
> @@ -302,3 +306,11 @@
>
> def shouldInstall(self, filename):
> return filename.startswith("%s/" % self.version)
> +
> +
> +class UefiUpload(SigningUpload):
> + """Legacy UEFI Signing custom upload."""
May be worth a slightly longer explanation of why this exists (preferably in
the description part of a multi-line docstring; see PEP 257), since presumably
we eventually want to be able to remove publication support for the old type
somewhere down the line, but it will likely be years away so we should record
institutional memory in the codebase.
> + custom_type = "raw-uefi"
See above.
> +
> + def distsDirectory(self):
> + return 'uefi'
>
> === modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
> --- lib/lp/archiveuploader/nascentuploadfile.py 2016-05-06 09:16:30
> +0000
> +++ lib/lp/archiveuploader/nascentuploadfile.py 2016-05-26 15:57:07
> +0000
> @@ -309,7 +313,8 @@
> """Return whether this custom upload can be automatically
> approved."""
> # Signing uploads will be signed, and must therefore be approved
> # by a human.
> - if self.custom_type == PackageUploadCustomFormat.SIGNING:
> + if self.custom_type in (PackageUploadCustomFormat.UEFI,
> + PackageUploadCustomFormat.SIGNING):
Ugly and somewhat confusing indentation. Prefer one of:
if self.custom_type in (PackageUploadCustomFormat.UEFI,
PackageUploadCustomFormat.SIGNING):
...
if self.custom_type in (
PackageUploadCustomFormat.UEFI,
PackageUploadCustomFormat.SIGNING):
...
> return False
> return True
>
>
> === modified file 'lib/lp/soyuz/scripts/custom_uploads_copier.py'
> --- lib/lp/soyuz/scripts/custom_uploads_copier.py 2016-05-06 09:16:30
> +0000
> +++ lib/lp/soyuz/scripts/custom_uploads_copier.py 2016-05-26 15:57:07
> +0000
> @@ -63,7 +67,8 @@
> return True
> # Signing uploads will be signed, and must therefore be approved
> # by a human.
> - if custom.customformat == PackageUploadCustomFormat.SIGNING:
> + if custom.customformat in (PackageUploadCustomFormat.UEFI,
> + PackageUploadCustomFormat.SIGNING):
Indentation: see above.
> return False
> return True
>
--
https://code.launchpad.net/~apw/launchpad/signing-reinstate-raw-uefi-custom-upload/+merge/295848
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
