Re: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor

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

 



On Tue, 2020-04-14 at 10:17 +0200, Erik Skultety wrote:
> On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote:
> > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
> > > +++ b/guests/playbooks/update/tasks/gitlab.yml
> > > +- name: Make {{ gitlab_runner_config_dir }} world readable
> > > +  file:
> > > +    path: '{{ gitlab_runner_config_dir }}'
> > > +    mode: '0755'
> > > +
> > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
> > > +  file:
> > > +    path: '{{ gitlab_runner_config_dir }}/config.toml'
> > > +    mode: '0644'
> > 
> > The message for these tasks is unnecessarily detailed: I'd just use
> > something like
> > 
> >   Make gitlab-runner configuration readable
> 
> Okay, however...
> 
> > for both.
> > 
> > Additionally, even though the gitlab user is going to be the only one
> > on the system so it doesn't make much of a difference in practice, I
> > think we should have config.toml
> > 
> 
> ...here you suggest the following adjustment. I feel like the messages above
> will then become confusing and misleading, since who are we making it readable
> for? Well, only for the gitlab user, so I think a little more detail in them is
> justifiable.
> 
> >   owner: root
> >   group: gitlab
> >   mode: '0640'
> 
> So how about:
> "Make gitlab-runner config dir readable" for the former and
> "Make gitlab-runner config.toml owned by the gitlab group" for the latter

I still think that's an unnecessary amount of detail, and we have
plenty of existing examples in the repository such as

  - name: Update installed packages
    package:
      name: fedora-gpg-keys
      state: latest
      disable_gpg_check: yes
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

  - name: Update installed packages
    command: '{{ package_manager }} update --refresh --exclude "kernel*" -y'
    args:
      warn: no
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

  - name: Update installed packages
    command: '{{ package_manager }} update --disablerepo="*" --enablerepo=fedora-rawhide-kernel-nodebug "kernel*" -y'
    args:
      warn: no
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

where we provide the high-level information as feedback to the user,
without going too much into detail - in this case, that we're
updating the system in three steps instead of a single one because
some packages require special handling.

But I don't want to hold up the series because of bikeshedding, so
if you are very keen on having the extra detail I'll take it as-is :)

-- 
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