Skip to content

fix: update edge_bundler_deno_v2 default value #6738

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 27, 2025
Merged

fix: update edge_bundler_deno_v2 default value #6738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccb1846
fix: update edge_bundler_deno_v2 default value
mrstork Oct 27, 2025
6f7da41
ci: update deno version used in workflows
mrstork Oct 27, 2025
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
8 changes: 3 additions & 5 deletions .github/workflows/workflow.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,12 @@ jobs:
os: [ubuntu-24.04, macos-14, windows-2025]
node-version: ['22']
# Must include the minimum deno version from the `DENO_VERSION_RANGE` constant in `node/bridge.ts`.
# We're adding v2.4.2 here because it's needed for the upcoming nimble release, so we can test
# those workflows ahead of time before we can update the base version across the board.
deno-version: ['v1.39.0', 'v2.2.4', 'v2.4.2']
deno-version: ['v2.4.2']
Comment on lines 56 to +57
Copy link
Contributor

@pieh pieh Oct 27, 2025

Choose a reason for hiding this comment

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

build/packages/edge-bundler/node/bridge.ts

Line 20 in 6f7da41

export const DENO_VERSION_RANGE = '1.39.0 - 2.2.4'

we still do allow deno 1 in allowed versions. The comment say we should include it? Or change the range in deno/bridge?

Copy link
Contributor Author

@mrstork mrstork Oct 27, 2025

Choose a reason for hiding this comment

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

Without changing the value in CI, I get all the test failures in the first commit and requires quite a few code changes in addition to the default flag value change. Changing the flag default is effectively changing the accepted range in deno/bridge but without removing the feature flag (since we would like to keep it around until the release is in the clear). The next release of build after this one will be removing the feature flag and cleaning up all the remaining code, but to keep the change set small in case a revert is needed, this was the path selected. Open to alternatives.

include:
- os: ubuntu-24.04
# Earliest supported version
node-version: '18.14.0'
deno-version: 'v2.2.4'
deno-version: 'v2.4.2'
fail-fast: false
steps:
# Sets an output parameter if this is a release PR
Expand Down Expand Up @@ -159,7 +157,7 @@ jobs:
- name: Setup Deno
uses: denoland/setup-deno@v1
with:
deno-version: v2.2.4
deno-version: v2.4.2
if: ${{ !steps.release-check.outputs.IS_RELEASE }}
- name: Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v6
Expand Down
2 changes: 1 addition & 1 deletion packages/edge-bundler/node/feature_flags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const defaultFlags = {
edge_bundler_generate_tarball: false,
edge_bundler_dry_run_generate_tarball: false,
edge_bundler_deno_v2: false,
edge_bundler_deno_v2: true,
}

type FeatureFlag = keyof typeof defaultFlags
Expand Down
Loading