search

collective / icalendar

icalendar parser library for Python
https://icalendar.readthedocs.io/en/latest/
Other
958 stars 166 forks source link

Use code styler #672

Open niccokunzmann opened 2 days ago

niccokunzmann commented 2 days ago

In order for the codebase to have a common style, we can use a linter/formatter like black or ruff.

While black allows us to format the codebase, ruff can enforce more compliance like import order, no prints and more.

To close this issue:

See also:

stevepiercy commented 2 days ago

This is a good unopinionated FAQ to help choose between tools: https://docs.astral.sh/ruff/faq/#how-can-i-disableforce-ruffs-color-output

I'd go with ruff, since we don't have a linter or formatter at this point, at least none that I know of.

I'd also suggest this issue be done before 6.0.0 final, because it will likely generate a huge diff.

document how to install the formatter/linter in a pre-commit hook. example

I don't see a .pre-commit-config.yaml file, so does this statement imply that there needs to be another step for developers in the documentation to install pre-commit?

niccokunzmann commented 1 day ago

I don't see a .pre-commit-config.yaml file, so does this statement imply that there needs to be another step for developers in the documentation to install pre-commit?

correct.

I am ok with ruff. If I create a PR, I am very happy to have contributions to it as it will be a lot of effort. I did it recently for the Open Web Calendar.

stevepiercy commented 1 day ago

I would also like contributions to provide a reason for using it as a comment in the configuration.

Line length on narrative text in documentation (not docstrings) does not make any sense, and it makes it difficult to do reviews due to rewrapping text to an arbitrary line length. Who cares? When rendered to HTML or other formats, line wraps go away. One sentence per line is a good guide, and what we use in Plone.

Please do begin with what you have from Open Web Calendar, and let's iterate over it. Thank you!

niccokunzmann commented 1 day ago

667 should be merged before and I would ask @stevepiercy, @geier, @angatha and @jacadzaca:

Would you have time to work on this together? When? This big diff will basically conflict with all PRs that are going on.

stevepiercy commented 1 day ago

I'd suggest start work on the configuration now, so we can see what both local testing and CI reveal without actually changing files. We can ignore local test and CI failures as "experimental" for a while. When there is an alpha release of 6.0.0, I assume that the PRs would slow down, then we would apply the configuration and write files in successive PRs, applying each format and lint configuration item separately.