Re: [libvirt] [jenkins-ci PATCH v2 2/2] projects: Document rationale for skipping jobs

2018-08-21 Thread Erik Skultety
On Tue, Aug 21, 2018 at 01:40:49PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-08-21 at 13:21 +0200, Erik Skultety wrote:
> > On Tue, Aug 21, 2018 at 12:38:06PM +0200, Andrea Bolognani wrote:
> > > In general, we strive for full coverage and build all
> > > projects on all targets; however, in some cases that's
> > > simply not feasible and we have to skip the corresponding
> > > job. Document the rationale for each such case.
> > >
> > > Signed-off-by: Andrea Bolognani 
> > > ---
> > >  projects/libvirt-dbus.yaml| 3 +++
> > >  projects/libvirt-sandbox.yaml | 3 +++
> > >  projects/libvirt-tck.yaml | 2 ++
> > >  projects/libvirt.yaml | 2 ++
> > >  projects/virt-manager.yaml| 1 +
> > >  projects/virt-viewer.yaml | 2 ++
> > >  6 files changed, 13 insertions(+)
> > >
> > > diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
> > > index fdfb615..459bd96 100644
> > > --- a/projects/libvirt-dbus.yaml
> > > +++ b/projects/libvirt-dbus.yaml
> > > @@ -2,6 +2,7 @@
> > >  - project:
> > >  name: libvirt-dbus
> > >  machines:
> >
> > I'd appreciate an empty line above the comments, it's adds to the 
> > readability.
> >
> > > +  # Debian 8 doesn't have a recent enough GLib
> > >- libvirt-centos-7
> > >- libvirt-debian-9
> > >- libvirt-fedora-27
>
> Existing comments don't have that, and in general the job definitions

Oh, well, I must have missed the few occurrences, never mind, it doesn't bother
me much really.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [jenkins-ci PATCH v2 2/2] projects: Document rationale for skipping jobs

2018-08-21 Thread Andrea Bolognani
On Tue, 2018-08-21 at 13:21 +0200, Erik Skultety wrote:
> On Tue, Aug 21, 2018 at 12:38:06PM +0200, Andrea Bolognani wrote:
> > In general, we strive for full coverage and build all
> > projects on all targets; however, in some cases that's
> > simply not feasible and we have to skip the corresponding
> > job. Document the rationale for each such case.
> > 
> > Signed-off-by: Andrea Bolognani 
> > ---
> >  projects/libvirt-dbus.yaml| 3 +++
> >  projects/libvirt-sandbox.yaml | 3 +++
> >  projects/libvirt-tck.yaml | 2 ++
> >  projects/libvirt.yaml | 2 ++
> >  projects/virt-manager.yaml| 1 +
> >  projects/virt-viewer.yaml | 2 ++
> >  6 files changed, 13 insertions(+)
> > 
> > diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
> > index fdfb615..459bd96 100644
> > --- a/projects/libvirt-dbus.yaml
> > +++ b/projects/libvirt-dbus.yaml
> > @@ -2,6 +2,7 @@
> >  - project:
> >  name: libvirt-dbus
> >  machines:
> 
> I'd appreciate an empty line above the comments, it's adds to the readability.
> 
> > +  # Debian 8 doesn't have a recent enough GLib
> >- libvirt-centos-7
> >- libvirt-debian-9
> >- libvirt-fedora-27

Existing comments don't have that, and in general the job definitions
are very "vertically compact" (there are pretty much no empty lines),
I'm not sure it would fit very well. Syntax highlighting should also
make it pretty much a non-issue.

Putting the comment on the line *after* 'machines:' instead of the
one *before* it was a mistake, though, and I'll make sure to fix it.

[...]
> >  - project:
> >  name: libvirt-sandbox
> > +# libvirt-sandbox is Linux only; among Linux platforms, CentOS 7 has
> > +# to be excluded because it doesn't ship a version of xz suitable for
> > +# statically linking against
> 
> Hmm, for some reason this doesn't sound "English" to me, how about:
> 
> s/statically linking against/linking statically/

Yeah, that's indeed much better :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [jenkins-ci PATCH v2 2/2] projects: Document rationale for skipping jobs

2018-08-21 Thread Erik Skultety
On Tue, Aug 21, 2018 at 12:38:06PM +0200, Andrea Bolognani wrote:
> In general, we strive for full coverage and build all
> projects on all targets; however, in some cases that's
> simply not feasible and we have to skip the corresponding
> job. Document the rationale for each such case.
>
> Signed-off-by: Andrea Bolognani 
> ---
>  projects/libvirt-dbus.yaml| 3 +++
>  projects/libvirt-sandbox.yaml | 3 +++
>  projects/libvirt-tck.yaml | 2 ++
>  projects/libvirt.yaml | 2 ++
>  projects/virt-manager.yaml| 1 +
>  projects/virt-viewer.yaml | 2 ++
>  6 files changed, 13 insertions(+)
>
> diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
> index fdfb615..459bd96 100644
> --- a/projects/libvirt-dbus.yaml
> +++ b/projects/libvirt-dbus.yaml
> @@ -2,6 +2,7 @@
>  - project:
>  name: libvirt-dbus
>  machines:

I'd appreciate an empty line above the comments, it's adds to the readability.

> +  # Debian 8 doesn't have a recent enough GLib
>- libvirt-centos-7
>- libvirt-debian-9
>- libvirt-fedora-27
> @@ -17,6 +18,7 @@
>parent_jobs: 'libvirt-glib-master-build'
>- autotools-syntax-check-job:
>parent_jobs: 'libvirt-dbus-master-build'
> +  # The test suite uses Python 3, which CentOS 7 doesn't include
>machines:
>  - libvirt-debian-9
>  - libvirt-fedora-27
> @@ -26,6 +28,7 @@
>  - libvirt-freebsd-11
>- autotools-check-job:
>parent_jobs: 'libvirt-dbus-master-syntax-check'
> +  # syntax-check uses Python 3, which CentOS 7 doesn't include
>machines:
>  - libvirt-debian-9
>  - libvirt-fedora-27
> diff --git a/projects/libvirt-sandbox.yaml b/projects/libvirt-sandbox.yaml
> index da9bf77..6a474c8 100644
> --- a/projects/libvirt-sandbox.yaml
> +++ b/projects/libvirt-sandbox.yaml
> @@ -1,6 +1,9 @@
>
>  - project:
>  name: libvirt-sandbox
> +# libvirt-sandbox is Linux only; among Linux platforms, CentOS 7 has
> +# to be excluded because it doesn't ship a version of xz suitable for
> +# statically linking against

Hmm, for some reason this doesn't sound "English" to me, how about:

s/statically linking against/linking statically/

Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [jenkins-ci PATCH v2 2/2] projects: Document rationale for skipping jobs

2018-08-21 Thread Andrea Bolognani
In general, we strive for full coverage and build all
projects on all targets; however, in some cases that's
simply not feasible and we have to skip the corresponding
job. Document the rationale for each such case.

Signed-off-by: Andrea Bolognani 
---
 projects/libvirt-dbus.yaml| 3 +++
 projects/libvirt-sandbox.yaml | 3 +++
 projects/libvirt-tck.yaml | 2 ++
 projects/libvirt.yaml | 2 ++
 projects/virt-manager.yaml| 1 +
 projects/virt-viewer.yaml | 2 ++
 6 files changed, 13 insertions(+)

diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
index fdfb615..459bd96 100644
--- a/projects/libvirt-dbus.yaml
+++ b/projects/libvirt-dbus.yaml
@@ -2,6 +2,7 @@
 - project:
 name: libvirt-dbus
 machines:
+  # Debian 8 doesn't have a recent enough GLib
   - libvirt-centos-7
   - libvirt-debian-9
   - libvirt-fedora-27
@@ -17,6 +18,7 @@
   parent_jobs: 'libvirt-glib-master-build'
   - autotools-syntax-check-job:
   parent_jobs: 'libvirt-dbus-master-build'
+  # The test suite uses Python 3, which CentOS 7 doesn't include
   machines:
 - libvirt-debian-9
 - libvirt-fedora-27
@@ -26,6 +28,7 @@
 - libvirt-freebsd-11
   - autotools-check-job:
   parent_jobs: 'libvirt-dbus-master-syntax-check'
+  # syntax-check uses Python 3, which CentOS 7 doesn't include
   machines:
 - libvirt-debian-9
 - libvirt-fedora-27
diff --git a/projects/libvirt-sandbox.yaml b/projects/libvirt-sandbox.yaml
index da9bf77..6a474c8 100644
--- a/projects/libvirt-sandbox.yaml
+++ b/projects/libvirt-sandbox.yaml
@@ -1,6 +1,9 @@
 
 - project:
 name: libvirt-sandbox
+# libvirt-sandbox is Linux only; among Linux platforms, CentOS 7 has
+# to be excluded because it doesn't ship a version of xz suitable for
+# statically linking against
 machines:
   - libvirt-debian-8
   - libvirt-debian-9
diff --git a/projects/libvirt-tck.yaml b/projects/libvirt-tck.yaml
index c406fda..38f8c09 100644
--- a/projects/libvirt-tck.yaml
+++ b/projects/libvirt-tck.yaml
@@ -1,6 +1,8 @@
 
 - project:
 name: libvirt-tck
+# CentOS 7 doesn't include perl-generators, which is necessary to
+# build libvirt-tck
 machines:
   - libvirt-debian-8
   - libvirt-debian-9
diff --git a/projects/libvirt.yaml b/projects/libvirt.yaml
index c64ac5b..ce4698e 100644
--- a/projects/libvirt.yaml
+++ b/projects/libvirt.yaml
@@ -10,6 +10,8 @@
   parent_jobs:
   - autotools-syntax-check-job:
   parent_jobs: 'libvirt-master-build'
+  # We limit syntax-check to Linux platforms because it calls some
+  # commands with more arguments than FreeBSD supports
   machines:
 - libvirt-centos-7
 - libvirt-debian-8
diff --git a/projects/virt-manager.yaml b/projects/virt-manager.yaml
index 84a95a6..6f806af 100644
--- a/projects/virt-manager.yaml
+++ b/projects/virt-manager.yaml
@@ -1,6 +1,7 @@
 
 - project:
 name: virt-manager
+# virt-manager is Python 3 only, so it can't be built on CentOS 7
 machines:
   - libvirt-debian-9
   - libvirt-fedora-27
diff --git a/projects/virt-viewer.yaml b/projects/virt-viewer.yaml
index e68a23b..9188239 100644
--- a/projects/virt-viewer.yaml
+++ b/projects/virt-viewer.yaml
@@ -13,6 +13,8 @@
   parent_jobs: 'virt-viewer-master-syntax-check'
   - autotools-rpm-job:
   parent_jobs: 'virt-viewer-master-check'
+  # The spec file for virt-viewer requires a very recent version
+  # of spice-gtk, so we have to skip this job on older distros
   machines:
 - libvirt-fedora-28
 - libvirt-fedora-rawhide
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list