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

Change type for variables to bool #132

Closed
wants to merge 4 commits into from
Closed

Change type for variables to bool #132

wants to merge 4 commits into from

Conversation

ppieprzycki
Copy link
Contributor

This commit will change variables commit_changes and replace_config to bool.
Also according to the doc string replace_config variable is not required but has default set to false.

After this change module will accept following values for mentioned variables:
yes,on,1,true,1,no,off,0,false,0

After this change module will not be backward compatible if someone used values "True" or "False" as in following example

msg: value of replace_config must be one of: yes,on,1,true,1,no,off,0,false,0, got: False

@dbarrosop
Copy link
Member

Hi!
The docstring mentions True and False for those parameters. Could you update the docstring to reflect what you mention in your PR message?

Thanks!

@ppieprzycki
Copy link
Contributor Author

Just updated doctoring.
Added all possible combinations as choices.

@dbarrosop
Copy link
Member

Thanks! Will do some tests soon and approve the PR if I don't see anything wrong (I a sure I wont but better safe than sorry). I am still enjoying some days off so I guess I will take a look early next week : )

This was referenced Jan 6, 2016
@dbarrosop
Copy link
Member

Hi! As you probably read here:

https://groups.google.com/forum/#!topic/napalm-automation/DI2w4s1vk9I

We have changed the organization and moved the ansible plugins to a dedicated repo. Would you mind closing this PR and sending a new one to the new repo? The reason why I prefer to do this is so your contribution is not lost during the movement.

@ppieprzycki
Copy link
Contributor Author

New pull request in separate repository
napalm-automation/napalm-ansible#2

@ppieprzycki ppieprzycki closed this Jan 7, 2016
dbarrosop added a commit that referenced this pull request Oct 20, 2017
Detect the format of the config and try loading it
dbarrosop pushed a commit that referenced this pull request Oct 22, 2017
napalm-nxos 0.7.0 release
dbarrosop pushed a commit that referenced this pull request Oct 23, 2017
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.

2 participants