On Thu, Mar 26, 2020 at 06:06:37PM +0100, Andrea Bolognani wrote:
> On Thu, 2020-03-26 at 12:35 +0000, Daniel P. Berrangé wrote:
> [...]
> > +for sha, subject in commits:
> > +
> > + msg = subprocess.check_output(["git", "show", "-s", sha],
> > + universal_newlines=True)
> > + lines = msg.strip().split("\n")
> > +
> > + print("🔍 %s %s" % (sha, subject))
>
> I could personally live without the emoji...
They are to make the important lines stand out more from the
rest of the log messages related to the CI job.
>
> > + sob = False
> > + for line in lines:
> > + if "Signed-off-by:" in line:
> > + sob = True
> > + if "localhost" in line:
> > + print(" ❌ FAIL: bad email in %s" % line)
> > + errors = True
>
> ... but if you absolutely must have them, at least don't try to mess
> with indentation - aligning text and emoji is basically never going
> to work reliably anyway. Please consider applying the diff at the
> end of this message if you think dropping the emoji is not an option.
I wasn't actually trying to align them - the fail lines are
intentionally indented, though not by enough.
>
> Anyway, the rest looks good, so as long as you at least remove that
> leading whitespace
>
> Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
>
>
> diff --git a/scripts/require-dco.py b/scripts/require-dco.py
> index 3b642d6679..cb057e48b3 100755
> --- a/scripts/require-dco.py
> +++ b/scripts/require-dco.py
> @@ -54,18 +54,20 @@ for sha, subject in commits:
> universal_newlines=True)
> lines = msg.strip().split("\n")
>
> - print("🔍 %s %s" % (sha, subject))
> + print(" %s %s " % (sha, subject), end="")
> sob = False
> for line in lines:
> if "Signed-off-by:" in line:
> sob = True
> if "localhost" in line:
> - print(" ❌ FAIL: bad email in %s" % line)
> + print("❌ (FAIL: bad email in %s)" % line)
> errors = True
>
> if not sob:
> - print(" ❌ FAIL missing Signed-off-by tag")
> + print("❌ (FAIL: missing Signed-off-by tag)")
This puts all the messages on one line, which results in long
lines when the commit message is already long. This is why
I put the fails on separate indented lines below the check
message.
> errors = True
> + else:
> + print("✅")
>
> if errors:
> print("""
> --
> Andrea Bolognani / Red Hat / Virtualization
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
