* gitlab-ci check-dco test
@ 2021-04-10 2:58 Richard Henderson
2021-04-10 9:03 ` Philippe Mathieu-Daudé
2021-04-12 9:05 ` Daniel P. Berrangé
0 siblings, 2 replies; 4+ messages in thread
From: Richard Henderson @ 2021-04-10 2:58 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
On development branches, it's not uncommon to push
temporary --fixup patches, and normally one doesn't
sign those. But then of course one get hate-mail
from the gitlab-ci job about the failing test.
Is there a way to make it fatal on staging, but
merely a warning on other branches (a-la checkpatch)?
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: gitlab-ci check-dco test
2021-04-10 2:58 gitlab-ci check-dco test Richard Henderson
@ 2021-04-10 9:03 ` Philippe Mathieu-Daudé
2021-04-12 8:59 ` Daniel P. Berrangé
2021-04-12 9:05 ` Daniel P. Berrangé
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-10 9:03 UTC (permalink / raw)
To: Richard Henderson; +Cc: Daniel P. Berrange, qemu-devel
On 4/10/21 4:58 AM, Richard Henderson wrote:
> On development branches, it's not uncommon to push
> temporary --fixup patches, and normally one doesn't
> sign those. But then of course one get hate-mail
> from the gitlab-ci job about the failing test.
>
> Is there a way to make it fatal on staging, but
> merely a warning on other branches (a-la checkpatch)?
To only run check-dco on branch /staging on any namespace:
-- >8 --
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3480d79db3a..f0d21da57f0 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -781,9 +781,9 @@ check-dco:
needs:
job: amd64-centos8-container
script: .gitlab-ci.d/check-dco.py
- except:
+ only:
variables:
- - $CI_PROJECT_NAMESPACE == 'qemu-project' && $CI_COMMIT_BRANCH ==
'master'
+ - $CI_COMMIT_BRANCH == 'staging'
variables:
GIT_DEPTH: 1000
---
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: gitlab-ci check-dco test
2021-04-10 9:03 ` Philippe Mathieu-Daudé
@ 2021-04-12 8:59 ` Daniel P. Berrangé
0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2021-04-12 8:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Richard Henderson, qemu-devel
On Sat, Apr 10, 2021 at 11:03:19AM +0200, Philippe Mathieu-Daudé wrote:
> On 4/10/21 4:58 AM, Richard Henderson wrote:
> > On development branches, it's not uncommon to push
> > temporary --fixup patches, and normally one doesn't
> > sign those. But then of course one get hate-mail
> > from the gitlab-ci job about the failing test.
> >
> > Is there a way to make it fatal on staging, but
> > merely a warning on other branches (a-la checkpatch)?
>
> To only run check-dco on branch /staging on any namespace:
>
> -- >8 --
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 3480d79db3a..f0d21da57f0 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -781,9 +781,9 @@ check-dco:
> needs:
> job: amd64-centos8-container
> script: .gitlab-ci.d/check-dco.py
> - except:
> + only:
> variables:
> - - $CI_PROJECT_NAMESPACE == 'qemu-project' && $CI_COMMIT_BRANCH ==
> 'master'
> + - $CI_COMMIT_BRANCH == 'staging'
> variables:
> GIT_DEPTH: 1000
Definitely don't want that - it skips the DCO check entirely on all
branches except 'staging'. We want contributors to see the missing
SoB on any of their branches *before* they send them to qemu-devel
at all.
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 :|
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: gitlab-ci check-dco test
2021-04-10 2:58 gitlab-ci check-dco test Richard Henderson
2021-04-10 9:03 ` Philippe Mathieu-Daudé
@ 2021-04-12 9:05 ` Daniel P. Berrangé
1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2021-04-12 9:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, Apr 09, 2021 at 07:58:48PM -0700, Richard Henderson wrote:
> On development branches, it's not uncommon to push
> temporary --fixup patches, and normally one doesn't
> sign those. But then of course one get hate-mail
> from the gitlab-ci job about the failing test.
>
> Is there a way to make it fatal on staging, but
> merely a warning on other branches (a-la checkpatch)?
Note, checkpatch is *never* fatal on any branch because there are always
scenarios in which checkpatch gives false positives that we have to allow.
We can of course set 'allow_failure: true' on the DCO check, on non-staging
branches, but that loose some of its value. In general I think users should
see this as a mandatory check, because we don't want them ever sending
patches without this passing. Your scenario of sometimes needing to push
temporary fix patches is valid too of course.
So this is a no-win scenario and we have to decide what the least worst
option is. When I added this check I decided the least worst was to have
developers see failures when they have temp fixup patches, because I was
optimizing for the ensuring developers see & fix problems before they
submit to qemu-devel.
In libvirt we have the same check, but we moved it to a separate stage
in the pipeline "sanity_checks" instead of "build", so developers can
see at a glance in the UI that the build jobs all passed, and only the
sanity check(s) failed.
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 :|
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-12 9:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-10 2:58 gitlab-ci check-dco test Richard Henderson
2021-04-10 9:03 ` Philippe Mathieu-Daudé
2021-04-12 8:59 ` Daniel P. Berrangé
2021-04-12 9:05 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).