-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci(check_ecosystem): add PyPa/build #3569
Conversation
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
@henryiii - Any ideas on how to resolve the above? |
It’s reading the pyproject.toml’s in test, I guess? Those need to be ignored. There are examples of broken toml files. Running it normally doesn’t have this issue. |
I do get it when running locally:
Does the typical invocation run over |
Typical invocation is via pre-commit, https://github.com/pypa/build/blob/e496fe6342f0e4729b69d0ea93eef529f27875a6/.pre-commit-config.yaml#L41. It probably passes all Python files explicitly. Maybe it doesn’t pick up nested pyproject.toml files in that case? |
Will it ignore them if I exclude tests/**/pyproject.toml? |
exclude = ["tests/packages/test-bad-syntax/pyproject.toml"] Doesn't seem to get |
I'll take a look, we need to exclude the entire directory so that it doesn't try to find the |
Omitting |
Yeah, I tried omitting the directory and the file. Don't want to omit |
Haha yeah I figured :) We have to load configuration files as we traverse, because the set of exclusions can change as we go deeper into the filesystem and find the relevant configuration files for each directory. So I'm guessing we're loading that config too eagerly somewhere despite the exclusion. |
I expect the exclusion still needs to be added to build. I can do that in a bit. |
🤦 Right right, sorry. A release isn't necessary, but the exclusion does need to be added. |
Why is it passing, actually? |
Adding PyPA/build.