-
Notifications
You must be signed in to change notification settings - Fork 31
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
Refactor build-release role tests.yml #432
Refactor build-release role tests.yml #432
Conversation
Signed-off-by: Maxwell G <gotmax@e.email>
Signed-off-by: Maxwell G <gotmax@e.email>
Automated with https://github.com/CentOS/infra-scripts/blob/master/ansible/convert-ansible-tasks-to-fqcn Signed-off-by: Maxwell G <gotmax@e.email>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! Would you mind adding a changelog fragment? Thanks.
roles/build-release/vars/main.yml
Outdated
antsibull_pip_bin: "{{ antsibull_ansible_venv }}/bin/pip" | ||
antsibull_pip_pkgs: "{{ _pip_pkgs['packages'][antsibull_pip_bin] }}" | ||
ansible_version_pypi: "{{ antsibull_pip_pkgs['ansible'][0]['version'] }}" | ||
_installed_ansible_core: "{{ antsibull_pip_pkgs['ansible-core'][0]['version'] }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about always using the prefix antsibull_ansible_venv
for everything that's in antsibull's ansible venv?
antsibull_pip_bin: "{{ antsibull_ansible_venv }}/bin/pip" | |
antsibull_pip_pkgs: "{{ _pip_pkgs['packages'][antsibull_pip_bin] }}" | |
ansible_version_pypi: "{{ antsibull_pip_pkgs['ansible'][0]['version'] }}" | |
_installed_ansible_core: "{{ antsibull_pip_pkgs['ansible-core'][0]['version'] }}" | |
antsibull_ansible_venv_pip_bin: "{{ antsibull_ansible_venv }}/bin/pip" | |
antsibull_ansible_venv_pip_pkgs: "{{ _pip_pkgs['packages'][antsibull_pip_bin] }}" | |
antsibull_ansible_venv_installed_ansible_version: "{{ antsibull_pip_pkgs['ansible'][0]['version'] }}" | |
antsibull_ansible_venv_installed_ansible_core_version: "{{ antsibull_pip_pkgs['ansible-core'][0]['version'] }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe antsibull_venv
? I dislike the long variable names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine for me. @dmsimard what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a changelog fragment. I've never written an ansible changelog fragment before, so let me know if it needs any changes :). |
db7de56
to
028b35f
Compare
changelogs/fragments/432-build-release-role-minor-refactoring.yml
Outdated
Show resolved
Hide resolved
0c431fd
to
9eff261
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gotmax23 Thanks for looking into this so quickly!
Suggesting minor changes to replace the left out short names with FQCN
Signed-off-by: Maxwell G <gotmax@e.email>
Signed-off-by: Maxwell G <gotmax@e.email> Co-authored-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Maxwell G <gotmax@e.email>
9eff261
to
70cfc21
Compare
changelogs/fragments/432-build-release-role-minor-refactoring.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: Maxwell G <gotmax@e.email>
Signed-off-by: Maxwell G <gotmax@e.email>
70cfc21
to
f3a9eaf
Compare
@gotmax23 thanks for contributing this! |
@Ompragash inspired me to look into how ansible is actually released, and I noticed some opportunities for improvement. Hopefully, this is helpful. At least, it has provided me with a distraction from my other work :).