-
Notifications
You must be signed in to change notification settings - Fork 601
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
Fixing releases to also mark SIPs as released #1546
Conversation
@@ -26,12 +26,12 @@ const finalizeRelease = async ({ layer, release, versionTag }) => { | |||
await versionsUpdate({ release, useOvm: true, versionTag }); | |||
} | |||
|
|||
const prerelease = semver.prerelease(versionTag) && semver.prerelease(versionTag) !== 'ovm'; | |||
const prerelease = semver.prerelease(versionTag) && semver.prerelease(versionTag)[0] !== 'ovm'; |
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 was the bug - semver.prerelease
was returning an array
Codecov Report
@@ Coverage Diff @@
## staging #1546 +/- ##
========================================
Coverage 95.92% 95.92%
========================================
Files 75 75
Lines 1743 1743
Branches 547 547
========================================
Hits 1672 1672
Misses 71 71 Continue to review full report at Codecov.
|
@@ -26,12 +26,12 @@ const finalizeRelease = async ({ layer, release, versionTag }) => { | |||
await versionsUpdate({ release, useOvm: true, versionTag }); | |||
} | |||
|
|||
const prerelease = semver.prerelease(versionTag) && semver.prerelease(versionTag) !== 'ovm'; | |||
const prerelease = semver.prerelease(versionTag) && semver.prerelease(versionTag)[0] !== 'ovm'; |
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.
are we certain that semver.prerelease(versionTag)[0]
is not null? otherwise the functionality would not be exactly the same as before.
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.
LGTM, just have same concern that db raised about being certain the first element in the array is not null
layer
-ovm
was being incorrectly skipped.