Parameter for Pyenv shellrc file named more descriptive #160
Conversation
Change-Id: Idf5a9420c8af521290244cc6f6d3829a71e4f07b
Change-Id: Ia4133e0824145a7263e0639b740a1ec92b1da84b
There was a problem hiding this comment.
Thanks again @MorganLindqvist for PR. I made two small suggestions based on syntax and lint checks for the changes you sent.
| when: pyenv_setting_path is defined | ||
|
|
||
| - name: "Load pyenv env variables in {{ pyenv_shellrc_file }}" | ||
| ansible.builtin.lineinfile: dest="{{ pyenv_shellrc_file }}" |
There was a problem hiding this comment.
Do you mind adding file permissions mode to this file? Ansible recommends explicits definition and in most cases I check what are the original permissions of the file.
There was a problem hiding this comment.
@staticdev This is not a row that I have added, only changed the name of the parameter. OK with any changes/improvements you would like to do in your code.
There was a problem hiding this comment.
@staticdev , looks like it would be quicker to approve this PR and then fix the issues in #161 . This PR does not introduce any new issues. The old ones are solved in #161 but that one needs github changes to make the automatic lint in github ehrn building the environment work.
| - name: "Add pyenv autocomplete in {{ pyenv_setting_path }}" | ||
| ansible.builtin.lineinfile: dest="{{ pyenv_setting_path }}" | ||
| - name: "Add pyenv autocomplete in {{ pyenv_shellrc_file }}" | ||
| ansible.builtin.lineinfile: dest="{{ pyenv_shellrc_file }}" |
There was a problem hiding this comment.
Another small improvement for consistency is to not use free-form parameters for actions, also ansible-lint favors thuis sintax.
Instead of:
ansible.builtin.lineinfile: dest="{{ pyenv_shellrc_file }}"
...
Use:
ansible.builtin.lineinfile:
dest: "{{ pyenv_shellrc_file }}"
...
There was a problem hiding this comment.
@staticdev This is not a row that I have added, only changed the name of the parameter. OK with any changes/improvements you would like to do in your code.
There was a problem hiding this comment.
@staticdev in the lint PR (#161 ) I fixed the above and all other lint warnings. When that one is merged I can rebase this. (or we can do it in the reverse order)
|
I merged this one so you may need to rebase the other. Thanks again @MorganLindqvist |
The parameter pyenv_setting_path is changed to pyenv_shellrc_file, which actually is what it is.
Added logic to keep backwards compability as well as print a deprecation warning. After suitable time the warning can be changed into a fail.