Re: [libvirt] [jenkins-ci PATCH v3 02/12] jobs: Remove archive_format from defaults
On Wed, 2018-08-22 at 12:37 +0200, Erik Skultety wrote:
On Wed, Aug 22, 2018 at 11:44:17AM +0200, Andrea Bolognani wrote:
> Instead of defining a default and overriding it on a
> case-by-case basis, always define archive_format at the
> project level.
>
> This adds a bit of duplication, but it's consistent with
> how we define other metadata for projects and it will
> help us out later.
How is it going to help us later?
Welp, I should definitely have expanded on those "will help us out
later" bits O:-)
So here you move something which is perfectly
fine to have in defaults and override it only when necessary and in the
following patch you're making a preparation changes for essentially the
opposite of this - moving autogen_args and similar to defaults (not that any
template needs to override these at the moment, but I'm getting confused by the
consistency you talk about).
The ultimate goal is to translate the JJB jobs definitions to
Ansible tasks while keeping the two as close as possible so that
further changes can be applied pretty much verbatim to both and
not make maintainance a nightmare.
Problem is, some JJB constructs are awfully difficult to translate
without adding extra boilerplate, so in those cases I opted for
moving to a different construct instead.
So for example autogen_args can be defined as a default because
every time we need to override it we can use
- autotools-build-job:
autogen_args: --enable-gtk-doc
which translates quite naturally to
- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
vars:
autogen_args: --enable-gtk-doc
since variables passed to an included task are limited in scope
to the included task and don't affect any other module call, but
for archive_format we can't do the same because we want to
translate
- project:
name: libvirt-dbus
archive_format: xz
to
- set_fact:
name: libvirt-dbus
archive_format: xz
and set_fact affects the *global* state, which means that it will
overwrite the default every time it is called and subsequent tasks
might break depending on the order they're called in.
I guess I could create a task that takes care of stashing the
existing defaults before overriding them with the
project-appropriate ones, and another one that restores the
previous values after running all tasks for a project, but as I
said that's extra boilerplate that we can avoid thanks to this
patch.
(As an aside, I actually like having archive_format spelled out
explicitly along with other project metadata such as name, title
and git_url: it makes sense to me to have that kind of information
all in one place. But the above is the actual reason why this
patch is needed.)
--
Andrea Bolognani / Red Hat / Virtualization