Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .craft.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
minVersion: 0.23.1
changelogPolicy: auto
postReleaseCommand: bash scripts/post-release.sh
targets:
- name: gcs
bucket: sentry-sdk-assets
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ jobs:
test_node:
name: Test Node
uses: ./.github/workflows/test_node.yml
with:
triggered-by-release: ${{ github.event_name == 'push' && startsWith(github.ref_name, 'release/') }}

test_swift:
name: Test Swift
Expand Down
33 changes: 25 additions & 8 deletions .github/workflows/test_node.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ name: Test Node

on:
workflow_call:
inputs:
triggered-by-release:
type: boolean
description: Whether the workflow was triggered by a release
default: false
outputs:
matrix-result:
description: 'Matrix job result'
Expand All @@ -19,10 +24,16 @@ jobs:
with:
node-version-file: package.json

# We need to skip the fallback download because downloading will fail on release branches because the new version isn't available yet.
# We have to use npm here because yarn fails on the non-existing existing optionalDependency version:
# https://github.com/yarnpkg/berry/issues/2425#issuecomment-1627807326
- run: SENTRYCLI_SKIP_DOWNLOAD=1 npm ci
- name: Install dependencies via npm ci
if: ${{ !inputs.triggered-by-release }}
run: npm ci
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Also, do we still need the SENTRYCLI_SKIP_DOWNLOAD env variable check at all anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the environment variable is unnecessary when providing --ignore-scripts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably could remove the check, only problem is we advertise it as public API in the README.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, didn't know this was mentioned in documentation. then let's leave it as-is. Thanks!


# For pushes to the release branch, we need to install the dependencies via `npm install`
# because the `package-lock.json` is not updated with the new versions of the optional
# dependencies yet. We also must skip the fallback download via --ignore-scripts.
- name: Install dependencies via npm install (for pushes to release branch)
if: ${{ inputs.triggered-by-release }}
run: npm install --omit=optional --ignore-scripts

- run: npm run check:types

Expand All @@ -43,10 +54,16 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

# We need to skip the fallback download because downloading will fail on release branches because the new version isn't available yet.
# We have to use npm here because yarn fails on the non-existing existing optionalDependency version:
# https://github.com/yarnpkg/berry/issues/2425#issuecomment-1627807326
- run: SENTRYCLI_SKIP_DOWNLOAD=1 npm ci
- name: Install dependencies via npm ci
if: ${{ !inputs.triggered-by-release }}
run: npm ci

# For pushes to the release branch, we need to install the dependencies via `npm install`
# because the `package-lock.json` is not updated with the new versions of the optional
# dependencies yet. We also must skip the fallback download via --ignore-scripts.
- name: Install dependencies via npm install (for pushes to release branch)
if: ${{ inputs.triggered-by-release }}
run: npm install --omit=optional --ignore-scripts

# older node versions need an older nft
- run: SENTRYCLI_SKIP_DOWNLOAD=1 npm install @vercel/nft@0.22.1
Expand Down
22 changes: 22 additions & 0 deletions scripts/post-release.sh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to make a release to properly test this. I plan to do immediately after merging

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

# This script is run by Craft after a release is created.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here and here for more info

# We currently use it to bump the platform-specific optional dependencies to their new versions
# in the package-lock.json, immediately after a release is created. This is needed for CI to
# pass after the release is created.c

set -eux
OLD_VERSION="${1}"
NEW_VERSION="${2}"

git checkout master

# We need to update the package-lock.json to include the new version of the optional dependencies.
npm install --package-lock-only

git add package-lock.json

# Only commit if there are changes
git diff --staged --quiet || git commit -m "build(npm): 🤖 Bump optional dependencies to ${NEW_VERSION}"
git pull --rebase
git push