search

vitabaks / postgresql_cluster

PostgreSQL High-Availability Cluster (based on "Patroni" and DCS "etcd" or "consul"). Automating with Ansible.
MIT License
1.29k stars 352 forks source link

Resolve linter issues (ansible-lint, yamllint, flake8) #315

Closed ThomasSanson closed 11 months ago

ThomasSanson commented 1 year ago

Close #308

ThomasSanson commented 1 year ago

make lint ok

vitabaks commented 1 year ago

@ThomasSanson consider offering a PR for yedit.py on the project repository

ThomasSanson commented 1 year ago

Hi @vitabaks, thank you for your comment.

I'm not quite sure if I understood your request correctly.

Are you suggesting that I should I cancel the changes I made in the current Pull Request ?

Please let me know so I can proceed accordingly. Thanks !

vitabaks commented 1 year ago

not necessarily cancel these changes.

It would just be nice to suggest improvements to the original repository of this module as well.

ThomasSanson commented 1 year ago

I don't understand the connection between this repository and ansible-role-yedit, except maybe that you were inspired by it ?

In any case, I need to take a closer look at how everything is organized and study it because submitting a pull request to ansible-role-yedit doesn't seem simple, as it doesn't appear to be welcoming to contributions at the moment. So, there's a lot of work to do.

Why didn't you include the role in a requirements.yml ?

I will create an issue on your repository so as not to forget

vitabaks commented 1 year ago

in fact, this is not a role, but a library for managing the yaml configuration, which you can observe here https://github.com/vitabaks/postgresql_cluster/tree/master/roles/patroni/library

I don't think it should be defined in requirements.yml

ThomasSanson commented 11 months ago

@vitabaks, I'm back from vacation ^^

I have just made the corrections for the merge conflicts, and everything is good to go for me to merge this pull request.

vitabaks commented 11 months ago

@ThomasSanson Welcome back! I'm glad you're here)

please don't forget to remove the skip_list rules from .ansible-lint based on what has been fixed.

To have a clear understanding of what else needs attention.

ThomasSanson commented 11 months ago

Hi @vitabaks,

Thank you for your comment.

I understand your concern regarding the skip_list rules in .ansible-lint. For now, I have just made the linters pass for this pull request.

My plan is to create a separate issue for each rule in skip_list after this pull request. This will allow us to address each issue systematically and have a clear understanding of what else needs attention.

I hope this addresses your concerns. Thank you again for your feedback.