Re: [libvirt-python PATCH 2/3] gitlab: introduce CI jobs testing git master & distro libvirt
On Thu, 2020-04-30 at 12:59 +0100, Daniel P. Berrangé wrote:
On Thu, Apr 30, 2020 at 10:16:32AM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-29 at 14:36 +0100, Daniel P. Berrangé wrote:
> > + before_script:
> > + - export TAG="${CI_REGISTRY_IMAGE}/buildenv-${NAME}:latest"
> > + - export
COMMON_TAG="${CI_REGISTRY}/libvirt/libvirt-perl/buildenv-${NAME}:latest"
>
> Since we're now storing images directly under the project's namespace
> on registry.gitlab.com and not somewhere more public like Quay, where
> people could conceivably look for a container image that can be used
> to *run* libvirt and friends instead of build them, I think we can
> safely drop the buildenv- prefix.
The GitLab container registry is just as public, so I think the
rationale still stands. Especially since when users submit a
merge request, these containers are going to magically appear
in their fork. I think can simplify the prefix to "ci-" hough.
Sounds good.
> > +.build_dist_job_template: &build_dist_job_definition
> > + stage: build
> > + before_script:
> > + - export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
> > + - export TEST_MAINTAINER=1
>
> Is TEST_MAINTAINER a thing for libvirt-python? The only reference to
> it I could find in the libvirt-ci repo is in the job definition for
> libvirt-perl.
I have another change for python that uses it, but havent sent
it yet.
Okay, so maybe introduce its use in GitLab CI at the same time as
you add support for it in the project proper.
Additional thing that I failed to mention the first time around: you
only set this environment variable in build_dist_job_template, but
you should probably have it in build_git_job_template as well.
> > +ctr-centos-7:
> > + <<: *container_job_definition
> > + variables:
> > + NAME: centos-7
>
> Not to keen on the ctr- prefix. I understand you're trying to keep
> the job names short so that they all fit in the tiny boxes in the
> pipeline overview page, but I don't think that should be an absolute
> requirement considering that the first thing you're going to do when
> you see a failure there is to click on the name of the job to see
> more details, and when you do so the full name will be displayed
> quite prominently in the top-right corner in a way that's actually
> readable, unlike the cut-off low-constrast version that's on the
> pipeline overview page...
I'll just switch to a "-container" suffix instead, so the most
important info is first.
Sounds good.
Please rename the build jobs accordingly at the same time.
--
Andrea Bolognani / Red Hat / Virtualization