Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Use
vcs_version
for Pants' version #20438base: main
Are you sure you want to change the base?
Use
vcs_version
for Pants' version #20438Changes from 1 commit
64b67dc
29e086f
b7c7c23
b3c1b16
7a2ad13
ccef094
db28c1c
d76b253
e8fb280
a0ca6ab
39fb59e
e8f887c
4b5ce76
e1ed9af
7bad224
397b016
b77b227
6cee103
dbff533
1148177
f6b37d9
bcbef9c
2676f8e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to computing the version 'accurately' when running from sources? What's the downside if we just substituted in a fixed dummy version here, and thus avoided needing to pull in
setuptools_scm
?In particular, I wonder if Pants supports running from a git-less tarball currently... if so, requiring
setuptools_scm
and its git interactions might break it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In those cases (which we don't document supporting currently) you're outside the normal flow. in that case, you'd just set
_PANTS_VERSION_OVERRIDE
on invocation (among other hacks, I imagine)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and I guess to answer your questions. I'd rather err on the side of caution. Where we could reasonably ask users to run
--version
and it gives us something useful.Then, if users want to exist outside of this model, they can do so by either setting the env var, or modifying the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also toying with speeding up option registration by doing it at build time instead of runtime, and in that case it would have to be tied to a specific version, and it would be chaotic to not invalidate that correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so I'm clear, is that something that'll preclude and block this PR? Or is it something that'll be handled in yours? Or something else?
Just trying to understand the impact of your statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just providing a reason that every version should have a well-defined version, even when running from sources, rather than using a fixed dummy.
This file was deleted.