[libvirt PATCH v3 11/12] gitlab: introduce a check for validate DCO sign-off

[Daniel P. Berrangé]

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 at redhat.com>
> 
> 
> 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 :|