search

ansible-collections / ansible-consul

:satellite: Ansible role for Hashicorp Consul clusters
https://galaxy.ansible.com/ansible-community/consul/
BSD 2-Clause "Simplified" License
456 stars 316 forks source link

Improve linting #520

Closed ppuschmann closed 1 year ago

ppuschmann commented 1 year ago

This PR is an updated version of #514 .

Please tell me to copy with 

Error: [E106] Role name ansible-consul does not match ``^[a-z][a-z0-9_]+$`` pattern
Error: [E102] No Jinja2 in when

or please solved it yourselves.

ppuschmann commented 1 year ago

Okay, I now fixed 

Error: [E102] No Jinja2 in when

with 14b0c7e and also applied f683264 to sort out the other error.

ppuschmann commented 1 year ago

How can we solve this? https://github.com/ansible-community/ansible-consul/actions/runs/3868366806/jobs/6593756679#step:4:455

ppuschmann commented 1 year ago

Maybe #521 should be merged earlier.

vitabaks commented 1 year ago

How can we solve this? https://github.com/ansible-community/ansible-consul/actions/runs/3868366806/jobs/6593756679#step:4:455

@ppuschmann

Add cgroupns_mode: host to molecule.yml And change /sys/fs/cgroup:/sys/fs/cgroup:ro to /sys/fs/cgroup:/sys/fs/cgroup:rw

https://github.com/ansible-community/molecule/pull/3665

My example: https://github.com/vitabaks/postgresql_cluster/commit/42b56fd17f0c2e6b9829c18a2afc526e5318a94a

nre-ableton commented 1 year ago

@ppuschmann So as you might have seen, I've just updated CI to use the latest ansible-lint version, which should improve the linting situation. However, we are now suppressing a number of new warnings, and these will need to fixed as well. I opened up a separate issue to track that, please see https://github.com/ansible-community/ansible-consul/issues/530.

Please rebase your changes and fix the merge conflicts. I only had a quick look at this PR, but it seems that some of the fixes are related to that PR while others are not (such as the capitalization stuff, which was also bugging me :smile:). After rebasing, ping me and I'll review this PR.

bbaassssiiee commented 1 year ago

I fixed some debian/ubuntu scenarios in https://github.com/ansible-community/ansible-consul/pull/534

bbaassssiiee commented 1 year ago

Thanks for your work, but I'd prefer more granular pull-requests because each review takes less time and head space. You could git cherry-pick and push separate branches.

ppuschmann commented 1 year ago

@bbaassssiiee @nre-ableton Thank you very much for your work.

I'll re-open new PR with more granular changes.

nre-ableton commented 1 year ago

@ppuschmann If you are eager to fix linting issues, please have a look at the TODO section in .ansible.lint: https://github.com/ansible-community/ansible-consul/blob/04f9975381bc2ffff60ec0f5455005b78c30436f/.ansible-lint#L3-L14

All of the above violations should be removed. Most of them are not difficult to fix.