eellak / epidose

Privacy-preserving epidemic dosimeter based on DP-3T contact tracing
Apache License 2.0
52 stars 6 forks source link

Add Ansible-lint suggested fixes #72

Closed stefanos1316 closed 3 years ago

stefanos1316 commented 3 years ago

I thought of adding a pro-hook commit to perform Ansible-lint. However, it requires the Ansible version of 2.9 at least. This version cannot be installed with apt but with pip. Therefore, if we want to install it with pip, we need to install python-pip too.

dspinellis commented 3 years ago

Well done! Regarding the installation of Ansible, can't it be specified as a Python module dependency together with the others we already have?

stefanos1316 commented 3 years ago

Do you mean that we should install the Ansible for the venvtoo? In that case, one needs to commit while having the venv activated in order to use the pre-commit hook of Ansible-lint which feels a bit weird, no? Note that we install the Python modules using the Ansible.

dspinellis commented 3 years ago

I was thinking to install a more recent version of ansible and ansible lint in venv (as a development dependency) in order to run ansible lint.

stefanos1316 commented 3 years ago

I think now I get what do you mean. You are saying that we should make the pre-commit hook to use the Ansible-lint that is installed in the venv, right?

dspinellis commented 3 years ago

Exactly!

23 Dec 2020 01:01:37 Stefanos Georgiou notifications@github.com:

I think now I get what you mean. You are saying that we should make the pre-commit hook to use the Ansible-lint that is installed in the venv, right?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub[https://github.com/eellak/epidose/pull/72#issuecomment-749823677], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAJICLC7F7365Q2PJHSPIHLSWEQMZANCNFSM4U5VIYBQ]. [https://github.com/notifications/beacon/AAJICLHRV666T5DKCSVGELDSWEQMZA5CNFSM4U5VIYB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFSYWNPI.gif]