-
Notifications
You must be signed in to change notification settings - Fork 14
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 new CRTM version v2.4_jedi #156
Add new CRTM version v2.4_jedi #156
Conversation
@BenjaminTJohnson FYI - please take a look and let me know what you think. Thanks! |
@@ -41,5 +41,8 @@ class Crtm(CMakePackage): | |||
version("2.4.0", commit="a831626") | |||
# Uses the tip of REL-2.3.0_emc branch | |||
version("2.3.0", commit="99760e6") |
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.
Assuming these version values are tags or should be consistent with tags, this should be "v2.4.0_emc" and "v2.3.0_emc"
v2.4.0_emc
tag has the commit: "5ddd0d6"
v2.3.0_emc
tag has the commit: "99760e6"
v2.3-jedi.4
is correct
v2.4_jedi
should be "0ee3593"
Not sure why there's a discrepancy between internal and public repos on the commit hashes for the tags. I'll investigate, but for now, let's use what's on public only.
Let me know if I'm misunderstanding something.
Thanks, you are right and I was wrong. Will make those changes.
… On Aug 19, 2022, at 4:35 PM, Benjamin Johnson ***@***.***> wrote:
@BenjaminTJohnson requested changes on this pull request.
In var/spack/repos/builtin/packages/crtm/package.py <#156 (comment)>:
> @@ -41,5 +41,8 @@ class Crtm(CMakePackage):
version("2.4.0", commit="a831626")
# Uses the tip of REL-2.3.0_emc branch
version("2.3.0", commit="99760e6")
Assuming these version values are tags or should be consistent with tags, this should be "v2.4.0_emc" and "v2.3.0_emc"
v2.4.0_emc tag has the commit: "5ddd0d6"
v2.3.0_emc tag has the commit: "99760e6"
v2.3-jedi.4 is correct
v2.4_jedi should be "0ee3593"
Not sure why there's a discrepancy between internal and public repos on the commit hashes for the tags. I'll investigate, but for now, let's use what's on public only.
Let me know if I'm misunderstanding something.
—
Reply to this email directly, view it on GitHub <#156 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RIBCNQ6IRXOTXEUCBLV2AD43ANCNFSM57B73CNQ>.
You are receiving this because you authored the thread.
|
I am building all four versions of CRTM on my macOS now, and I found a bug or two - will update the PR in a bit. |
…ing dependencies for release v2.4_jedi
@srherbener @BenjaminTJohnson I was able to install all four versions of CRTM on my macOS after the latest bug fixes that I just pushed. |
Since @BenjaminTJohnson is on leave for three weeks and I used his corrected hashes, and since the PR was approved and everything built as expected, I am going to merge this once the unit tests passed. |
As the title says - also update the list of maintainers to more accurately reflect who is maintaining the CRTM package in spack, and fix the URL to point to the authoritative CRTM repository.
See #155 for a detailed description.
Fixes #155