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

Create a top-level pants_release package. #19297

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Jun 12, 2023

Move release.py and the files it imports from build-support
into the new package.

build-support was originally intended for lightweight scripts,
but it now contains non-trivial code that imports other code
from build-support.

Unfortunately, since build-support/bin is a source root, this
means that the imports aren't namespaced under anything
pants-specific. This change fixes that.

Move release.py and the files it imports
from build-support into the new package.

build-support was originally intended for
lightweight scripts, but it now contains
non-trivial code that imports other code
from build-support. Unfortunately, since
build-support/bin is a source root, this means
that the imports aren't namespaced under
anything pants-specific. This change fixes that.
@benjyw benjyw added the category:internal CI, fixes for not-yet-released features, etc. label Jun 12, 2023
@kaos
Copy link
Member

kaos commented Jun 12, 2023

there's also the idea to keep the code in build-support/bin/pants to have both the namespacing and the separation from the "real" pants code.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 12, 2023

there's also the idea to keep the code in build-support/bin/pants to have both the namespacing and the separation from the "real" pants code.

A) I would argue this is real code
B) How would we make sure names under this pants don't collide with names in src/python/pants? We'd have to insist on a unique subpackage, like pants.release, at which point why not put it under src/python to enforce that?
C) This is still separated in its own top-level package

We discourage our users from artificially splitting up the repo into multiple source roots, since Pants provides the tooling to not have to do that. I suggest we follow our own advice!

BTW, I would still prefer src/python/pants/release as, again, this is real code. But I can tolerate src/python/pants_release.

Happy to bikeshed that last point though.

@cognifloyd
Copy link
Member

Should this be pants_release or something like pants_build_support which hints at its intended usage in the build-support directory?

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 12, 2023

"release" is succinct and fairly descriptive. "build-support" is a rather fuzzy term, since all of Pants is for "build support", it's not obvious that this means "support for building pants itself". And in any case, build-support is in fact about releasing...

@benjyw benjyw merged commit f9df772 into pantsbuild:main Jun 12, 2023
25 checks passed
@benjyw benjyw deleted the pants_release_pkg branch June 12, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants