Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: #39: draft of local config #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vmindru
Copy link
Contributor

@vmindru vmindru commented Jan 23, 2018

this is proposal for #39

@willthames
Copy link
Owner

As discussed elsewhere, I think if we augment ansible.cfg with an [ansible-review] section, that should be relatively flexible.

@vmindru
Copy link
Contributor Author

vmindru commented Jan 25, 2018 via email

@willthames
Copy link
Owner

I actually created an .ansible-review directory today with a config file in it - this would have saved me passing the extra -c. I might test this in the next couple of days with a view to getting it into 0.14.0

I realise you're likely long since past caring - I hope you are well though!

@vmindru
Copy link
Contributor Author

vmindru commented Aug 24, 2019

Hi :) doing fine I actually still do care, it just possible got off my radar.

@vmindru
Copy link
Contributor Author

vmindru commented Aug 24, 2019

I was thinking of how to implement this and started with smthing like.

 36     local_config = os.path.join(os.getcwd(), '.ansible-review', 'config.ini')$   
 37     if os.path.isfile(ANSIBLE_CFG):$                                             
 38         print("config found")$                                                   
 39         sys.exit(1)$                                                             
 40     if os.path.isfile(local_config):$                                            
 41         default_config_file = local_config$                                      
 42     else:$                                                                       
 43         config_dir = AppDirs("ansible-review", "com.github.willthames").user_config_dir$
 44         default_config_file = os.path.join(config_dir, "config.ini")$   ```

the problem here is that we can not know for suere where config.ini will be located, is it home folder, is it default /etc/ansible/ansible.cfg etc etc, figuring out which config to use can end up being trying to be too smart with the tool and getting into user's way. I opt for leaving this as original PR 

36 local_config = os.path.join(os.getcwd(), '.ansible-review', 'config.ini')$
37 if os.path.isfile(local_config):$
38 default_config_file = local_config$
39 else:$
40 config_dir = AppDirs("ansible-review", "com.github.willthames").user_config_dir$
41 default_config_file = os.path.join(config_dir, "config.ini")$ ````

@willthames
Copy link
Owner

I think the PR as it stands looks pretty good. I'd just like to test it - I'll try and do that next week

Copy link

@ansiblejunky ansiblejunky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally and works, but why must we have a folder .ansible-review? I understand we can put the lint rules in the folder too, but can we also allow the config file to be .ansible-review with a folder at all? I prefer not having my rules inside each repo... otherwise I have to update the rules in all of my ansible repos one at a time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants