Skip to content
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

Add pants_requirements target generator and deprecate pants_requirement macro #13512

Merged
merged 8 commits into from Nov 24, 2021

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Nov 6, 2021

Progress towards #12915.

This new target generator is much more useful. It leans into the reality that Pants 2 only releases pantsbuild.pants and pantsbuild.pants.testutil, and that almost always you want to use both when developing plugins. It also removes irrelevant fields like dist and modules.

The version is also more flexible now, as described in the new help message.

--

Note that this is our first time we're using target generator syntax dir:tgt#gen in non-Go code. Even though #12917 is not decided, there seems to be consensus on using target generator syntax for file-less targets like python_requirement.

# 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]
# 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]
Comment on lines 88 to 89
# Macros
*pants_requirements.rules(),
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question that I don't know the answer to: should this be in a separate backend? The implication would be that the first step of plugin development would be "enable to the plugin_development backend" or something. But it might avoid some confusion of having it listed in the python backend as a whole.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I like that. We have a tonnnn of targets right now with ./pants help.

But hey, that makes me wonder, why even have pantsbuild.pants.testutil anymore?? I theoretically get the argument that pantsbuild.pants should be as small as possible for end-users. But in reality, pantsbuild.pants.testutil is only 23.2kb. I'll bring this to Slack

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answered in Slack. It's useful to have pantsbuld.pants.testutil (or the extra pantsbuild.pants[testutil]) so that our test support can depend on extra third-party requirements. At that point, we can stick with the two dists because it's roughly a different typing of the single dist with an extra.

So, pants_requirements still makes sense. We only need to create a dedicated backend.

@kaos
Copy link
Member

kaos commented Nov 15, 2021

Would <3 to see this land... :)

Eric-Arellano added a commit that referenced this pull request Nov 24, 2021
Prework for #13512. We need to stop using `pants_requirement`.

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor Author

@benjyw or @kaos , can you please ack that I'm adding pants.backend.plugin_development? For now, it only registers pants_requirements target generator. In the future, we could maybe add other helpful tools.

# 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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants