mbox series

[0/3] gitlab: add jobs for checking paches

Message ID 20200918132903.1848939-1-berrange@redhat.com (mailing list archive)
Headers show
Series gitlab: add jobs for checking paches | expand

Message

Daniel P. Berrangé Sept. 18, 2020, 1:29 p.m. UTC
This introduces two new jobs to GitLab. The first runs "checkpatch.pl"
across all patches, while the second is a dedicated DCO signoff check.

While checkpatch.pl does validate DCO signoff, it is sub-optimal as we
need to allow the checkpatch.pl job to fail as there are always patches
which intentionally violate some rules, and we've no mechanism for marking
permitted exceptions in code. Thus the checkpatch.pl jobs needs to be
non-fatal allowing failure.

By having a separate DCO job, we can make that particular job mandatory.

Checking patches themselves in GitLab CI is a little difficult, as the
CI job receives no indication of what the base ancestor was for the
branch being tested. To work around this, we add the master QEMU git
repo as a new remote and ask git to find the common ancestor vs the
branch being tested.

An example pipeline showing failure of these two jobs is here:

  https://gitlab.com/berrange/qemu/-/pipelines/191219666

The checkpatch.pl job failure output:

  https://gitlab.com/berrange/qemu/-/jobs/743439455

And the DCO signoff job failure output:

  https://gitlab.com/berrange/qemu/-/jobs/743439456

I think the latter shows the benefit of having a dedicated DCO signoff
job checker, as the info presented to the user is much clearer about
what they did wrong and how & why they must address it.

_+#          base:  master

Daniel P. Berrangé (3):
  gitlab: add a CI job for running checkpatch.pl
  gitlab: add a CI job to validate the DCO sign off
  gitlab: assign python helper files to GitLab maintainers section

 .gitlab-ci.d/check-dco.py   | 94 +++++++++++++++++++++++++++++++++++++
 .gitlab-ci.d/check-patch.py | 48 +++++++++++++++++++
 .gitlab-ci.yml              | 22 +++++++++
 MAINTAINERS                 |  1 +
 4 files changed, 165 insertions(+)
 create mode 100755 .gitlab-ci.d/check-dco.py
 create mode 100755 .gitlab-ci.d/check-patch.py

Comments

no-reply@patchew.org Sept. 18, 2020, 1:36 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20200918132903.1848939-1-berrange@redhat.com/



Hi,

This series failed build test on FreeBSD host. Please find the details below.






The full log is available at
http://patchew.org/logs/20200918132903.1848939-1-berrange@redhat.com/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org Sept. 18, 2020, 1:54 p.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/20200918132903.1848939-1-berrange@redhat.com/



Hi,

This series failed build test on FreeBSD host. Please find the details below.






The full log is available at
http://patchew.org/logs/20200918132903.1848939-1-berrange@redhat.com/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Thomas Huth Sept. 18, 2020, 2:07 p.m. UTC | #3
On 18/09/2020 15.29, Daniel P. Berrangé wrote:
> This introduces two new jobs to GitLab. The first runs "checkpatch.pl"
> across all patches, while the second is a dedicated DCO signoff check.

This feels quite redundant since we're checking the patches with Patchew
already ... or are there plans to get rid of this check in Patchew?

 Thomas
Philippe Mathieu-Daudé Sept. 18, 2020, 2:10 p.m. UTC | #4
On 9/18/20 4:07 PM, Thomas Huth wrote:
> On 18/09/2020 15.29, Daniel P. Berrangé wrote:
>> This introduces two new jobs to GitLab. The first runs "checkpatch.pl"
>> across all patches, while the second is a dedicated DCO signoff check.
> 
> This feels quite redundant since we're checking the patches with Patchew
> already ... or are there plans to get rid of this check in Patchew?

The plan is to use GitLab-CI gating :)

Also this free patchew community resources and use the user's
resources instead. From a patchew sysadmin PoV this is a win
IMO.

> 
>  Thomas
>
Daniel P. Berrangé Sept. 18, 2020, 2:15 p.m. UTC | #5
On Fri, Sep 18, 2020 at 04:07:22PM +0200, Thomas Huth wrote:
> On 18/09/2020 15.29, Daniel P. Berrangé wrote:
> > This introduces two new jobs to GitLab. The first runs "checkpatch.pl"
> > across all patches, while the second is a dedicated DCO signoff check.
> 
> This feels quite redundant since we're checking the patches with Patchew
> already ... or are there plans to get rid of this check in Patchew?

patchew only runs once the contributor has sent their patches to the
mailing list, whci his too late.

We want contributors to test their series in GitLab CI ahead of sending
it, so that patchew never has to report any failure, because the code is
already perfect once on the list (except if git master has moved and
causes conflicts of course).

Regards,
Daniel
Yonggang Luo Sept. 18, 2020, 2:19 p.m. UTC | #6
On Fri, Sep 18, 2020 at 10:16 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
>
> On Fri, Sep 18, 2020 at 04:07:22PM +0200, Thomas Huth wrote:
> > On 18/09/2020 15.29, Daniel P. Berrangé wrote:
> > > This introduces two new jobs to GitLab. The first runs "checkpatch.pl"
> > > across all patches, while the second is a dedicated DCO signoff check.
> >
> > This feels quite redundant since we're checking the patches with Patchew
> > already ... or are there plans to get rid of this check in Patchew?
>
> patchew only runs once the contributor has sent their patches to the
> mailing list, whci his too late.
>
> We want contributors to test their series in GitLab CI ahead of sending
> it, so that patchew never has to report any failure, because the code is
> already perfect once on the list (except if git master has moved and
> causes conflicts of course).
>
> 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 :|
>
>
agreed, and ineed patchew are broken now.

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo
no-reply@patchew.org Sept. 18, 2020, 2:57 p.m. UTC | #7
Patchew URL: https://patchew.org/QEMU/20200918132903.1848939-1-berrange@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.






The full log is available at
http://patchew.org/logs/20200918132903.1848939-1-berrange@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com