Re: [jenkins-ci PATCH v2 10/12] lcitool: Add projects information handling

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

 



On Tue, Jul 17, 2018 at 04:44:24PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-07-17 at 15:04 +0200, Katerina Koukiou wrote:
> > On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote:
> > > +class Projects:
> > > +
> > > +    def __init__(self):
> > > +        try:
> > > +            with open("./vars/mappings.yml", "r") as f:
> > 
> > There is clear information where how to run the lcitool in the docs
> > in some patches befor so the relative paths that are used everywhere in
> > the code are not causing a problem.
> > Though IMO, I think it's clearer to have a variable (config
> > option, hardcoded, env variable or whatever you decide), storing the path of
> > these files so that this code is not dependent on relative paths. WDYT?
> 
> Some of the paths, like vars/mappings.yml, are pretty much entirely
> arbitrary but others, like group_vars/all/main.yml, can't be changed
> because that's what Ansible expects.
> 
> Additionally, none of the paths is repeated more than once in the
> script so using something like
> 
>   mappings_path = "./vars/mappings.yml"
>   open(mappings_path, "r")
> 
> instead of
> 
>   open("./vars/mappings.yml", "r")
> 
> wouldn't IMHO buy us much.

I meant only having a variable for the playbook location.

playbook_path = os.getenv('PLAYBOOK_PATH', './'))

And then use os.path.join for all the others relative paths to the
playbook path.
So that you can actually run the lcitool script from wherever you want.

Anyway, since this script usage is not wide, feel free to keep
everything hardcoded.

> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization

Attachment: signature.asc
Description: PGP signature

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

[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