-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[exprtk] Add versions 0.0.2 #42996
base: master
Are you sure you want to change the base?
[exprtk] Add versions 0.0.2 #42996
Conversation
The pipeline errors will be fixed by PR #42912. |
@LilyWangLL thanks for the update on the pipeline issue. Please feel free to merge this PR, once #42912 and the current PR's pipeline goes all green 🟢 |
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 PR will now only contain tag 0.0.2 instead of both 0.0.2 and 0.0.3
In fact, version 2023-01-01 does not exist in vcpkg. This PR directly upgraded from version 2022-01-01 to version 2024-01-01. |
24fc06d
to
6367623
Compare
The commit for version 2023-01-01 is just a historical submission in this PR. The final data merged in the PR reflects the final changes, just from version 2022-01-01 to version 2024-01-01. After this PR is merged, users will only be able to install versions 2022-01-01 and 2024-01-01 in vcpkg, and not version 2023-01-01. The data in this JSON file needs to correspond to the changes shown in the image below and should not include any additional modifications. |
If you want both versions 2023-01-01 and 2024-01-01 to be installable, you need to submit separate PRs for each version. After version 2023-01-01 is merged, then merge version 2024-01-01. |
@LilyWangLL ok, I'll convert this PR to 2023 and then create another one once this has been merged for 2024, are you ok with me proceeding? |
That's great! |
6367623
to
0ec126a
Compare
@LilyWangLL updated the PR to only include 0.0.2 Please merge the PR once the pipeline issues have been resolved. |
@@ -1,7 +1,6 @@ | |||
{ | |||
"name": "exprtk", | |||
"version-date": "2022-01-01", | |||
"port-version": 2, | |||
"version-date": "2023-01-01", |
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 port should finally decide which version scheme to use. The subject says 0.0.2
, and there is also a tag for 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.
@ArashPartow we should be using the version scheme that upstream uses. In this case, it appears there's a tag for 0.0.3
or use the tag that corresponds with this version, 0.0.2
?
REF F46BFFCD6966D38A09023FB37BA9335214C9B959 | ||
SHA512 5D4F1CC49E035860D9B2410A9B48E1ADF8784C6301A31B2C7462C2DBFA64CA6763A832354E57414919DF7B7D23975F5F7759D0984BECAF25B39FB0AEDE6B716F |
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.
Nitpick: In vcpkg ports, we usually see lower case for these values.
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.
Is there a formal definition anywhere regarding this? I checked other ports and it seems to be a mixed bag. It will be uppercase for this port.
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.
There is more like a dominant inflow than a mixed bag.
git
uses lower case.
sha512sum
produces lower case (on linux).
cmake -E sha512sum
produces lower case.
vcpkg hash
produces lower case. (IIRC it was deliberately change to do that. Does this count as a formal definition?)
@LilyWangLL can you please merge this PR, as I need to do the next one. |
Not resolved: Use lower-case SHA512 and git ref SHA, #42996 (comment). Still confusing: Versioning. |
@dg0yt I requested you provide a guideline that requires this. Please also note you have since the start of 2023 getting in the way of ExprTk being updated, with nitpicks etc. Can you please refrain from this PR unless you have serious and legitimate concerns.
Just a short sampling, but have you been involved in any of these?
when I originally created the port vcpkg for this library, I followed another port's scheme, because the library doesn't use semver. I've added semver recently to the GH repo to make it easier for people that use package managers. This was accepted at the time. So I'm not sure what is the best path going forward that wont break backwards compatibility. In short if you have a complete and viable solution please let me know. Otherwise you are always free to make your own PR once these have been merged and take over this port. |
@Cheney-W / @Mengna-Li / @PhoebeHui / @jimwang118 / @JonLiu1993 / @LilyWangLL Can any of you please merge this PR. Been trying to get it merged since early 2023 |
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.
See comments
@@ -1,8 +1,24 @@ | |||
Copyright 1999-2022 Arash Partow |
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 see that upstream has a license file. Please just install that copyright file rather than checking it in here.
@@ -1,7 +1,6 @@ | |||
{ | |||
"name": "exprtk", | |||
"version-date": "2022-01-01", | |||
"port-version": 2, | |||
"version-date": "2023-01-01", |
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.
@ArashPartow we should be using the version scheme that upstream uses. In this case, it appears there's a tag for 0.0.3
or use the tag that corresponds with this version, 0.0.2
?
./vcpkg x-add-version --all
and committing the result.Closes: #42173, #40270, #33571, #30058, #29955, #29801, #29775, #29665, #38756
@PhoebeHui / @jimwang118 / @JonLiu1993 / @LilyWangLL