-
Notifications
You must be signed in to change notification settings - Fork 83
feat: skip infoview breaking test on major version bump #459
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
base: master
Are you sure you want to change the base?
Conversation
It looks like the Windows CI is indeed able to execute the test as is. But maybe it uses WSL and the general user doesn't? |
| @@ -0,0 +1,18 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| PUBLISHED_VERSION="$(npm view @leanprover/infoview-api version --no-workspaces)" | |||
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 this supposed to be the infoview-api version?
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.
no it's supposed to be @leanprover/infoview, my bad!
|
|
||
| PUBLISHED_VERSION="$(npm view @leanprover/infoview-api version --no-workspaces)" | ||
| NEW_VERSION="$(sed -n 's/^\s*"version":\s*"\(.*\)",\s*/\1/p' ./package.json)" | ||
| PUBLISHED_MAJOR="$(cut -d '.' -f 1 <<< $PUBLISHED_VERSION)" |
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.
If the first component of the semantic version is 0, then the major version is the second component of the semantic version. This will yield a major version of 0 for 0.7.0, but it should be 7.
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 thouht semantic versioning is usually Major-Minor-Patch. is your scheme ...-Major-Minor, i.e would it be fine to always check the first two terms?
Can fix that tomorrow
Uh oh!
There was an error while loading. Please reload this page.