Re: [libvirt-python PATCH 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux