Pants internally uses dedicated Sources and Dependencies fields #16037
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]
| @@ -42,3 +44,35 @@ def pytest_sessionfinish(session) -> None: | |||
| # Technically unecessary, but nice if people are running tests directly from repo | |||
| # (not using pants). | |||
| namespace_init_path.unlink() | |||
|
|
|||
|
|
|||
| @pytest.fixture(autouse=True, scope="session") | |||
There was a problem hiding this comment.
This code should be fast enough, but it is worth noting we're being bit here by an issue our users have often brought up: scope is effectively impotent in the face of the way we run tests.
There was a problem hiding this comment.
Oh yeah, I just didn't want it firing for every test case when we could do better 🤷♂️
There was a problem hiding this comment.
Also scope isn't impotent. Session scope is though. The other scopes are legitimate
There was a problem hiding this comment.
Would an __init_subclass__ production runtime check be "better"? That would also loop in 3rdparty plugin authors.
There was a problem hiding this comment.
I thought about it, but decided to scope this to just our repo for now. I'd honestly approve that PR though.
We should be intentional though, so this is a good first step.
[ci skip-rust] [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
thejcannon
changed the title
There was a problem hiding this comment.
I think this is good as an "internal only change" -- it is not a Plugin API change. PR label should be updated imo.
I would be more concerned if it was a Plugin API change because it goes against our instructions in Target API Concepts, where we explain often there's zero need to subclass.
|
I marked it as such because can update their plugins to use more specific types if they so choose. |
|
It is a nonbreaking plugin API change IMO |
|
Okay, then maybe rename to "Pants internally uses dedicated Sources and Dependencies fields" |
thejcannon
changed the title
Fixes #15727. Note there's an auto-use fixture now which validates the convention. This is easier than a flake8 plugin.
[ci skip-rust]