ENH Add support for config files#69
Conversation
for more information, see https://pre-commit.ci
…int into support-config-files
for more information, see https://pre-commit.ci
…int into support-config-files
for more information, see https://pre-commit.ci
…int into support-config-files
for more information, see https://pre-commit.ci
…int into support-config-files
for more information, see https://pre-commit.ci
|
thanks for your PR! I think what we also need is some example of how to set configuration in the readme Unless there's an example of a package which uses cython-lint but can't use pyproject.toml, I'd suggest dropping setup.cfg, let's keep things simple |
| Search for a pyproject.toml or a setup.cfg file in common | ||
| parent directories of the given list of paths. | ||
| """ | ||
| paths = [path.resolve() for path in paths] |
There was a problem hiding this comment.
aren't we iterating over paths multiple times like this? can we just call resolve() directly one level up?
| if sys.version_info >= (3, 11): # pragma: no cover | ||
| import tomllib | ||
| else: | ||
| import tomli as tomllib |
There was a problem hiding this comment.
doesn't this need to be a runtime dependency now?
There was a problem hiding this comment.
I guess so, at least while py3.11 is not the min supported version :) . I added it to the install_requires section
| while root != root.parent: | ||
|
|
||
| # Look for pyproject.toml first | ||
| config_file = root / 'pyproject.toml' | ||
| if config_file.is_file(): | ||
| config = tomllib.loads(config_file.read_text()) | ||
| config = config.get('tool', {}).get('cython-lint', {}) | ||
| if config: | ||
| return config |
There was a problem hiding this comment.
does this logic come from some other tool? if so, it would be good to reference it. if not, we should add more extensive testing
There was a problem hiding this comment.
I tested the behavior of other linters like black and flake8. You don't have to run the tool from the root of your project and so they'll try to locate it by climbing up the path. I couldn't copy paste their logic because it's entangled in a much more complicated logic but I tried to mimic the behavior. I extended one of the tests in this regard.
…int into support-config-files
| args = parser.parse_args(argv) | ||
| paths = [pathlib.Path(path).resolve() for path in args.paths] | ||
|
|
||
| # Update defaults from pyproject.toml or setup.cfg if present |
There was a problem hiding this comment.
| # Update defaults from pyproject.toml or setup.cfg if present | |
| # Update defaults from pyproject.toml if present |
😉
Okay, let's keep it simple for now. We can add support for setup.cfg if there's demand |
|
🤔 not sure why half the jobs are cancelled without even starting |
|
Do you know why the ubuntu jobs are cancelled ? I just see them cancelled but no detail is given |
|
I don't know - want to try ubuntu-latest instead of ubuntu-18.04? |
|
I was looking at https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/ It seems that the image is deprecated and should have been removed a while ago :) |
|
awesome! just needs a little example in the readme, then we can merge
sure - OK if I add you as collaborator? |
|
Sure, no problem, thanks ! |
Fixes #64
This PR adds the possibility to set the configuration either from
pyproject.tomlor fromsetup.cfg.I chose to support
setup.cfgas well because I read a lot of discussion regarding the transition to pyproject.toml and it seems that it has not been adopted by a bunch of projects. The main reason I found was the issue that it automatically triggers isolated builds even when you want to install in editable mode. Sincecython-lintis used exclusively by projects that do have some compilation, I thought it was more convenient to supportsetup.cfg.