Merge ~paelzer/ubuntu-helpers:lp-test-small-fixes into ~paelzer/ubuntu-helpers:master

[Christian Ehrhardt ]

[Michał Małoszewski (michal-maloszewski99)]

Code is good in my opinion.
One question: I know it's not linux kernel coding style, but wouldn't it be easier to use tabs and then spaces to align the code?
I mean <tab> <space space space> code - instead of only spaces. I mean indentation.

[Christian Ehrhardt  (paelzer)]

On Wed, Jun 22, 2022 at 1:58 PM Michał Małoszewski
<email address hidden> wrote:
>
> Code is good in my opinion.
> One question: I know it's not linux kernel coding style, but wouldn't it be easier to use tabs and then spaces to align the code?
> I mean <tab> <space space space> code - instead of only spaces.

TBH I think the majority of code we deal with uses only space and we
generally stick to whatever we find some code use.

P.S: In fact I can't write tabs as I globally auto convert it to 4
spaces, to write one I have to copy it. Which confirms how rarely it
is needed.

[Bryce Harrington (bryce)]

On Wed, Jun 22, 2022 at 02:54:18PM -0000, Christian Ehrhardt  wrote:
> On Wed, Jun 22, 2022 at 1:58 PM Michał Małoszewski
> <email address hidden> wrote:
> >
> > Code is good in my opinion.
> > One question: I know it's not linux kernel coding style, but wouldn't it be easier to use tabs and then spaces to align the code?
> > I mean <tab> <space space space> code - instead of only spaces.
>
> TBH I think the majority of code we deal with uses only space and we
> generally stick to whatever we find some code use.
>
> P.S: In fact I can't write tabs as I globally auto convert it to 4
> spaces, to write one I have to copy it. Which confirms how rarely it
> is needed.

My own policy is to conform to the tab/space style of the project in
question. In my experience spaces-only cause the least commotion,
however I've worked on codebases the full spectrum and so have adopted
the habit of including a Mode line in my scripts. For python scripts I
include this as the 2nd line:

# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-

This both helps to document the whitespace convention for the project on
a per-file basis, and also is parsed by vim/emacs so I can be lazy and
not have to think about it. :-)

Bryce

[Bryce Harrington (bryce)]

Thanks for the review Christian! And doubly so for implementing your review comments and doing a follow on MP. Per your directions on https://code.launchpad.net/~paelzer/ubuntu-helpers/+git/ubuntu-helpers/+merge/425209 I've merged both branches and landed them.

I squashed some of the smaller and more obvious changes into the commits that created the issue, but have left most of your review comments as-is since they include some useful commentary and/or are distinct.

I need to re-base the branch Athos reviewed on top of these, however we're still discussing a couple refactoring points, so will tackle that later. Meanwhile, please let me know if you spot any other issues with any of these changes.

review: Approve

[Bryce Harrington (bryce)]

This branch can be marked merged btw.

diff --git a/cpaelzer/lp-test-ppa b/cpaelzer/lp-test-ppa
index 448ed70..91017ae 100755
--- a/cpaelzer/lp-test-ppa
+++ b/cpaelzer/lp-test-ppa
@@ -30,11 +30,14 @@ from launchpadlib.launchpad import Launchpad
 
 # Global constants
 ARCHES = ["amd64", "s390x", "ppc64el", "arm64", "armhf", "riscv64"]
+LPAPI = "https://api.launchpad.net/devel"
+
 
 def error(msg):
     """Prints message to stderr"""
     sys.stderr.write("Error: {}\n".format(msg))
 
+
 class Job:
     def __init__(self, number, time, pkg, series, arch, jobinfo):
         self.number = number
@@ -146,7 +149,7 @@ def showActive(release, ppa_user, ppa_name):
     if running:
         print(rformat % ("time", "pkg", "release", "arch", "ppa", "trigger"))
         for e in running:
-            print(rformat % (str(e.time), e.pkg, e.series, e.arch, ','.join(e.ppas), ','.join(e.triggers)))
+            print(rformat % (str(datetime.timedelta(seconds=e.time)), e.pkg, e.series, e.arch, ','.join(e.ppas), ','.join(e.triggers)))
 
     print("Waiting:")
     waiting = getWaiting()
@@ -290,11 +293,12 @@ def showResult(result, published):
 
         print("\t%s %s" % (overall_status, report_txt))
 
+
 def showBinaries(ppa, release):
     """Report all published binaries in this PPA"""
     print("Binaries:")
     for arch in ARCHES:
-        archs = "%s/ubuntu/%s/%s" % (base, release, arch)
+        archs = "%s/ubuntu/%s/%s" % (LPAPI, release, arch)
         ppa_builds = ppa.getPublishedBinaries(distro_arch_series=archs)
         if not ppa_builds:
             print("%s - no binaries present yet" % arch)
@@ -305,6 +309,7 @@ def showBinaries(ppa, release):
                                                build.binary_package_version,
                                                build.status))
 
+
 def create_arg_parser():
     """Sets up the command line parser object.
 
@@ -401,13 +406,14 @@ def main(args):
 
     # log in
     cred_location = os.path.expanduser('~/.lp_creds')
-    credential_store = UnencryptedFileCredentialStore(cred_location)
+    UnencryptedFileCredentialStore(cred_location)
     lp = Launchpad.login_with('affectrelease', 'production', version='devel')
 
     # project/person owning the PPA
     person = lp.people[ppa_user]
 
     if args.arches:
+        global ARCHES
         ARCHES = args.arches[0]
 
     # get ppa
@@ -421,8 +427,7 @@ def main(args):
         for release in args.releases[0]:
             print("---- ---- ---- ----")
             print("Release: %s" % release)
-            base="https://api.launchpad.net/devel"
-            series = "%s/ubuntu/%s" % (base, release)
+            series = "%s/ubuntu/%s" % (LPAPI, release)
             ppa_sources = ppa.getPublishedSources(distro_series=series)
             if not ppa_sources:
                 print("Warning: no sources present for %s" % release)
@@ -462,6 +467,7 @@ def main(args):
         error("PPA %s not found" % ppa_name)
         return 1
 
+
 if __name__ == "__main__":
     parser = create_arg_parser()
     args = parser.parse_args()