Clarify the coding style used + use automatic code formatting

[mhartmay]

To make the code base easier to maintain, and to make it easier for others to contribute, we should decide which coding style we should stick to and which tool we want to use to make formatting the code easier & enforce that coding style.

Suggestions:

• https://github.com/astral-sh/ruff
• https://github.com/psf/black
• https://github.com/google/yapf

---

Checklist:

• Modify code to be compliant with the new tool → Use of 'ruff' as linter and formatter #257
• Modify pre-commit files to use the new tool → Use of 'ruff' as linter and formatter #257
• Update the README file to suggest setting up pre-commit
• Update the CI to verify if PR are compliant

[matttbe]

Good idea to pick one. Because there are no stable versions, it should be easy to include a bit of code refactoring.

Personally, I'm not a Python expert, and I think I only used Black, and probably Ruff as well. But Ruff is also a linter, no? If Ruff is picked, maybe it should replace Flakes8 and Pylint? WDYT?

Having a tool that takes care of the code format is often good and useful, but I hope it will not block occasional contributors if there are too many things to setup before suggesting a small change.

I guess the idea is to have pre-commit doing all the required checks, and a verification done by the CI, right? Then a new dedicated section in the README about that should be added as well.

[SPYFF]

Ruff is linter and formatter or at least that what it claims. I'm using it in smaller project, but mostly for linting.

[mhartmay]

Good idea to pick one. Because there are no stable versions, it should be easy to include a bit of code refactoring.

Personally, I'm not a Python expert, and I think I only used Black, and probably Ruff as well. But Ruff is also a linter, no? If Ruff is picked, maybe it should replace Flakes8 and Pylint? WDYT?

Yep, but that's another topic we can discuss then.

Having a tool that takes care of the code format is often good and useful, but I hope it will not block occasional contributors if there are too many things to setup before suggesting a small change.

I wouldn't enforce the use of tools, but the way how the code is written/formatted (-> needs to be documented) :) And that's up to the developer if they want to use tooling for this or not... I would suggest to use tooling, but that's only my opinion.

I guess the idea is to have pre-commit doing all the required checks, and a verification done by the CI, right? Then a new dedicated section in the README about that should be added as well.

Yes, to make it easier for developers, but it's optional to use pre-commit, it's not enforced. As for the CI, theoretically we could even run pre-commit hooks in the CI, but I haven't thought about it in detail. Normally, I try to make us developers "happy" first and then the CI.

Ideally, if we have chosen a code formatter we create one large commit and add this commit rev. to a .git-blame-ignore-revs file (see https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt).

[matttbe]

Good idea to pick one. Because there are no stable versions, it should be easy to include a bit of code refactoring.
Personally, I'm not a Python expert, and I think I only used Black, and probably Ruff as well. But Ruff is also a linter, no? If Ruff is picked, maybe it should replace Flakes8 and Pylint? WDYT?

Yep, but that's another topic we can discuss then.

Yes, but that might be one reason to pick Ruff if it can replace other tools :)

Having a tool that takes care of the code format is often good and useful, but I hope it will not block occasional contributors if there are too many things to setup before suggesting a small change.

I wouldn't enforce the use of tools, but the way how the code is written/formatted (-> needs to be documented) :) And that's up to the developer if they want to use tooling for this or not... I would suggest to use tooling, but that's only my opinion.

Indeed, no need to enforce this. But I guess we also want to make sure the code is always correctly formatted, not to create other unwanted changes when modifying files that didn't have the correct format.

I guess the idea is to have pre-commit doing all the required checks, and a verification done by the CI, right? Then a new dedicated section in the README about that should be added as well.

Yes, to make it easier for developers, but it's optional to use pre-commit, it's not enforced. As for the CI, theoretically we could even run pre-commit hooks in the CI, but I haven't thought about it in detail. Normally, I try to make us developers "happy" first and then the CI.

Good idea to run the pre-commit hooks in the CI! I think it is important to keep all files with the correct format. If this check is executed when a PR is opened, then it should at least not block the occasional developers to open a PR.

Ideally, if we have chosen a code formatter we create one large commit and add this commit rev. to a .git-blame-ignore-revs file (see https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt).

Why not, just in case, even if there are no stable branches.
Also, Flakes8 and/or Pylint already enforces some typical Python styles, I guess there should not be too many modifications if we pick another tool.

Should we try Ruff? Do you plan to try it with vng?

[mhartmay]

I've created a first RFC PR - feedback is highly appreciated.

#257

What's currently is missing, is the coding style documentation somewhere...

Edit:
Regarding the CI, you might want to take a look at https://pre-commit.ci/ - it's free for open source projects. Note: I've never used it, just discovered it.

[matttbe]

I've created a first RFC PR - feedback is highly appreciated.

#257

Thank you for the PR and Andrea for the review!

What's currently is missing, is the coding style documentation somewhere...

I guess something could be added in the README file, no?

I just added a checklist in the first comment not to forget what still need to be done before closing this ticket.

Edit: Regarding the CI, you might want to take a look at https://pre-commit.ci/ - it's free for open source projects. Note: I've never used it, just discovered it.

Good idea, it seems very easy to setup, and it supports the auto-update. I guess only @arighi can do that if the repo is under his name. Andrea, what do you think about using this CI? If there is an issue, we can fallback to GitHub Actions later I suppose.

[arighi]

I agree about adding a comment in the README, maybe under the "Contributing" section?

About pre-commit, not sure, I'll take a look. If you guys think it's useful I'm fine to add that, but in general, I'd like to avoid making CI over complicated or add dependencies of external tools, everything should be self-consistent if possible.

[matttbe]

I agree about adding a comment in the README, maybe under the "Contributing" section?

Good idea!

@mhartmay just to avoid a deadlock, do you plan to update the README? :)

About pre-commit, not sure, I'll take a look. If you guys think it's useful I'm fine to add that, but in general, I'd like to avoid making CI over complicated or add dependencies of external tools, everything should be self-consistent if possible.

Personally, I would prefer if the CI can tell us when the files are no longer Ruff compliant: for those who setup Ruff in their editor and/or pre-commit, it will not be nice to get unintended changes after an auto-formatting, or having to deal with existing warnings before starting modifying files.

This should not block us merging PR, but good to know if something needs to be fixed later if needed. From what I understood, this CI just needs to be linked to a repo, nothing else to do. In this case, we could remove the pylint.yml file and simplify the CI maintenance :)

[mhartmay]

I agree about adding a comment in the README, maybe under the "Contributing" section?

Good idea!

@mhartmay just to avoid a deadlock, do you plan to update the README? :)

No, not yet. I'm currently implementing the virtiofs support for s390x in virtme-ng.

Would be great if someone else could do the update! Thanks in advance.

About pre-commit, not sure, I'll take a look. If you guys think it's useful I'm fine to add that, but in general, I'd like to avoid making CI over complicated or add dependencies of external tools, everything should be self-consistent if possible.

Personally, I would prefer if the CI can tell us when the files are no longer Ruff compliant: for those who setup Ruff in their editor and/or pre-commit, it will not be nice to get unintended changes after an auto-formatting, or having to deal with existing warnings before starting modifying files.

Yep. Shouldn't be too hard to implement via github actions, either with or without pre-commit. Most often, I try to maintain only one file (e.g. pre-commit configuration).

[mhartmay]

@arighi @matttbe Do we want to encourage the developers to use typing hints? If yes, we should probably use mypy, pyright or pyre.

[arighi]

Personally I'm not a big fan of typing hints, especially if they bring more dependencies... :)

Edit: but I don't have strong opinions, so if you guys think there's value in that I'm fine also to enforce type hints.

[septatrix]

Personally I'm not a big fan of typing hints, especially if they bring more dependencies... :)

Keep in mind that those are not runtime dependencies. I like the peace of mind they give and IDE completions are just a thousand times better if it knows that the variable is a dict or str, and not "Any"