ci(release): npm install in test_node.yml on release#2768
ci(release): npm install in test_node.yml on release#2768szokeasaurusrex merged 1 commit intomasterfrom
npm install in test_node.yml on release#2768Conversation
1aefb76 to
40bb062
Compare
| @@ -0,0 +1,22 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # This script is run by Craft after a release is created. | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
40bb062 to
b1d8933
Compare
There was a problem hiding this comment.
We will need to make a release to properly test this. I plan to do immediately after merging
npm install in bump-version.shnpm install in test_node.yml on release
|
You can see the checks passing on this release, which was generated against this PR. |
| - run: SENTRYCLI_SKIP_DOWNLOAD=1 npm ci | ||
| - name: Install dependencies via npm ci | ||
| if: ${{ !inputs.triggered-by-release }} | ||
| run: npm ci |
There was a problem hiding this comment.
didn't we previously just do SENTRYCLI_SKIP_DOWNLOAD=1 npm install? Seemed a bit simpler than the check and the conditional steps 😅
No need to change if this works, was just curious.
There was a problem hiding this comment.
l: Also, do we still need the SENTRYCLI_SKIP_DOWNLOAD env variable check at all anymore?
There was a problem hiding this comment.
didn't we previously just do SENTRYCLI_SKIP_DOWNLOAD=1 npm install? Seemed a bit simpler than the check and the conditional steps 😅
Yeah, I think that is why it broke now, but at the same time, I want the locking to avoid situations like we had a few days ago, with incompatible releases breaking our CI. Better to run CI against locked dependencies to make it reproducible, imo
There was a problem hiding this comment.
I think the environment variable is unnecessary when providing --ignore-scripts
There was a problem hiding this comment.
We probably could remove the check, only problem is we advertise it as public API in the README.md
There was a problem hiding this comment.
ah ok, didn't know this was mentioned in documentation. then let's leave it as-is. Thanks!
b1d8933 to
c37e733
Compare
`npm ci` will fail here, as the new versions of the optional dependencies are not published yet. Additionally, add a script to bump the optional dependencies in the package-lock.json after a release is created. Otherwise, `npm ci` will continue to fail after the release, until someone updates the package-lock.json manually.
c37e733 to
9826efe
Compare
`npm ci` will fail here, as the new versions of the optional dependencies are not published yet. Additionally, add a script to bump the optional dependencies in the package-lock.json after a release is created. Otherwise, `npm ci` will continue to fail after the release, until someone updates the package-lock.json manually.
npm ciwill fail here, as the new versions of the optional dependencies are not published yet.Additionally, add a script to bump the optional dependencies in the package-lock.json after a release is created. Otherwise,
npm ciwill continue to fail after the release, until someone updates the package-lock.json manually.