adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.87k stars 275 forks source link

build: add PEP-517 build config with Poetry #557

Closed staticdev closed 1 year ago

staticdev commented 1 year ago

Closes #509

adrienverge commented 1 year ago

Hello, thanks for tackling this!

The use of Poetry doesn't seem mandatory: could we use something minimal, even if files are not automatically generated? (For example @DimitriPapadopoulos mentioned keeping setuptools: is it an option?)

Also I see some problems, e.g. inconsistencies in Python version dependencies (3.6+ vs. 3.7+), no more command to build documentation, the duplication of the yamllint version in multiple files.

Finally, the impact of this to distro packaging (.rpm, .deb, etc.) should be kept as fluent as possible.

staticdev commented 1 year ago

Hello, thanks for tackling this!

The use of Poetry doesn't seem mandatory: could we use something minimal, even if files are not automatically generated? (For example @DimitriPapadopoulos mentioned keeping setuptools: is it an option?)

Also I see some problems, e.g. inconsistencies in Python version dependencies (3.6+ vs. 3.7+), no more command to build documentation, the duplication of the yamllint version in multiple files.

Finally, the impact of this to distro packaging (.rpm, .deb, etc.) should be kept as fluent as possible.

I tried in this PR to make a minimal change to be compliant with new PEP-517. But you have all valid points.

About setuptools.. It is possible to keep, but did not understand, why do you want to keep it?

About duplicated version.. New packages only use pyproject.toml to control the version. I did not remove it from init file for a simple reason that you still support python 3.7. From 3.8 it is just add importlib-metadata and you are good to go with version. There is also a backport for python 3.7 but all projects I work on already dropped 3.7 since it is 2 months for EOL https://endoflife.date/python

For documentation build I am not sure how you want to go. I usually just add a simple requirements.txt on my docs folder and build from there on github actions. You can also see as example any of my python repos.

I did not dig yet how you are doing other packages. Do you have any suggestions regarding the changes I did regarding that topic?

ssbarnea commented 1 year ago

I also dislike poetry and use setuptools-scm instead, I could probably make a PR that make uses of it.

staticdev commented 1 year ago

@adrienverge just in case I addressed in this PR the issue regarding docs build and version management (now it is only part of pyproject.toml as recommneded - it will only install importlib_metadata as dependency for Python <3.8). This is basically how it is working in all projects I am currently maintainer.

If setuptools is must then setuptools-scm from @ssbarnea suggestion would be the other approach.

DimitriPapadopoulos commented 1 year ago

I am under the impression that setuptools is not considered more standard than poetry nowadays. Yet, I would prefer setuptools too, together with _setuptoolsscm to extract the version.

staticdev commented 1 year ago

Actually I don't understand the preference on continuing to use setuptools. Can any of you enlighten me with a list features setuptools/setuptools_scm has that poetry does not have? Poetry is built from scratch upon the new standards of Python packaging defined:

Pros of poetry for me:

I don't really have cons.

DimitriPapadopoulos commented 1 year ago

Speaking for myself, I guess I am getting tired of the continuous changes and instability around Python. As if the necessary Python 2 to Python 3 migration was not enough, I now have to spend excessively much time adapting to continuous changes in the Python development and packaging environment. It is a complete mess – although I understand this is an effort to move from the current mess to a better future. As a result, I think I have a strong personal bias against such continuous breaking changes, and arguments in favour of such changes. That said, changes do happen nevertheless out there, I cannot ignore them for ever :smile:

adrienverge commented 1 year ago

Hello, and thanks for these valuable inputs!

I have more or less the same fears as @DimitriPapadopoulos (but maybe I'm wrong): Poetry is actively maintained, but what about in 10 years? I prefer stability over features. Yamllint dependencies are kept simple on purpose (just PyYAML + pathspec, always the last version): could a fully-featured dependency management tool like Poetry be overkill? That being said, I haven't followed the progress of each tool recently, so I may be mistaken.

In any case, the next packaging system should:

Would setuptools + setuptools_scm allow lighter configuration? Is it possible to avoid using poetry.lock (the project dependencies are simple and we stay up-to-date)? Is it possible to put .flake8 data inside pyproject.toml (like we did with setup.cfg)?

ssbarnea commented 1 year ago

Flake8 config is the only one that cannot be moved, mainly because current maintainer did not want to add support for it. Anyway, I think that flake8 days are numbered, as ruff is getting traction at huge speed. I started using it few days ago and I was amazed. I will be using it in parallel with flake8 for at least two months before removing flake8. Anyway, that is bit diverging from the subject but I imagined that you could find the information useful. And, yep, that one uses pyproject.toml.

adrienverge commented 1 year ago

@ssbarnea good to know, thanks :+1: I just tried it but it doesn't support configuration in setup.cfg. So it could be a good idea to switch from flake8 to ruff, but after the upgrade to pyproject.toml.

staticdev commented 1 year ago

Also interesting for me.. i will keep an eye on ruff.

UPDATE @adrienverge I fixed errors on CI/CD of missing dependency and also a replace.

adrienverge commented 1 year ago

Thanks for the updated code @staticdev :+1:

It looks like tests still won't start. In the meantime, I'll work on a setuptools version of pyproject.toml so we can compare more easily.

adrienverge commented 1 year ago

I opened https://github.com/adrienverge/yamllint/pull/565 and https://github.com/adrienverge/yamllint/pull/566 to upgrade from setup.py to pyproject.toml, using the setuptools way. I didn't include setuptools-scm for automatic versioning, to keep things more straightforward.

First of all, thanks for this Poetry version @staticdev, I appreciate the work done. But after testing with setuptools, I see multiple reasons to use the latter:

You can see how it looks here: https://github.com/adrienverge/yamllint/pull/566.

staticdev commented 1 year ago

@adrienverge I have rebased based on your changes. Could you run CI again?

coveralls commented 1 year ago

Coverage Status

Coverage: 99.196%. Remained the same when pulling b23f67361a128c139e988876d0a67ea3477898f6 on staticdev:feature/pyproject.toml into 16eae28a50047960b8841468ac7b8822c8d1cc66 on adrienverge:master.

staticdev commented 1 year ago

I think when you remove setup.py from your PR you will also need coverage[toml] otherwise it will not fetch config @adrienverge