-
Notifications
You must be signed in to change notification settings - Fork 519
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 Versioning Support from version.txt
#3140
base: develop
Are you sure you want to change the base?
Changes from 1 commit
5779779
104dfb0
cb849ee
0ce318c
0eff6a4
dd6e579
23b93e3
31e3692
d6be0ff
8bc99cd
2eb6c93
5219406
d4b37cf
160bf43
2b47355
923f84a
07c65a3
940c571
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ authors = [ | |
{name = "The OpenMC Development Team", email = "[email protected]"}, | ||
] | ||
description = "OpenMC" | ||
version = "0.15.1-dev" | ||
dynamic = ["version"] | ||
requires-python = ">=3.10" | ||
license = {file = "LICENSE"} | ||
classifiers = [ | ||
|
@@ -58,6 +58,9 @@ Documentation = "https://docs.openmc.org" | |
Repository = "https://github.com/openmc-dev/openmc" | ||
Issues = "https://github.com/openmc-dev/openmc/issues" | ||
|
||
[tool.setuptools.dynamic] | ||
version = {file = "tools/version.txt"} | ||
|
||
[tool.setuptools.packages.find] | ||
include = ['openmc*', 'scripts*'] | ||
exclude = ['tests*'] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
1.15.1-dev | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably beyond the scope of this PR alone, but this isn't PyPA compliant. It should be: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setuptools will handle the task for us, so we can continue using the legacy format. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something setuptools does because it's nice to do; not because it has to. I think changing to PyPA compliant is a small change that would reduce the risk of something breaking in the future. (Honestly I don't think many people would even notice beyond some of the core devs.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PyPA does allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. TIL.
ahnaf-tahmid-chowdhury marked this conversation as resolved.
Show resolved
Hide resolved
|
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.
Does this file need locating elsewhere, perhaps in the source code. I'm not sure but perhaps that helps when packaging as I don't think the tools folder is included in the packaging (e.g. conda)
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.
This file is only required at build time. If it is necessary at runtime, we can put it in the root dir.
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.
It appears that openmc-feedstock uses a specific release tar file, which includes all the folders, including the tools folder.
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.
Thanks for answering this
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 not an expert, but this doesn't feel like a
tools
sort of thing. To me it feels like a top level 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.
Doing it this way means
openmc.__version__
still works due to how__version__
is populated.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 have stored this file in the tools directory because it is only required during build time. Python's wheel uses it to create the wheel, and CMake uses it to generate the
version.h
file. Additionally, we can use this file for other purposes. I placed it in that folder to keep the root directory clean. However, if necessary, we can move it to the root directory.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 tend to agree that this is more of a root-level file.