Skip to content

Conversation

@jonrohan
Copy link
Member

@jonrohan jonrohan commented Nov 5, 2025

No description provided.

@jonrohan jonrohan requested a review from a team as a code owner November 5, 2025 18:14
@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

⚠️ No Changeset found

Latest commit: 6ca873d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates the canary release workflow into the main release workflow file and updates the Node.js version. The separate release_canary.yml file is removed, and both main and canary release jobs are now defined in a single release.yml file.

  • Merges canary release workflow into main release workflow
  • Updates Node.js version from 18 to 24
  • Adds a non-standard "repo" field to theme/package.json

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
.github/workflows/release_canary.yml Deleted the standalone canary release workflow
.github/workflows/release.yml Consolidated main and canary release jobs, updated Node version to 24, removed branch filters from trigger
theme/package.json Added non-standard "repo" field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -1,39 +1,82 @@
name: Release
on:
push:
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The workflow now triggers on every push to any branch without filtering. This will cause both jobs to evaluate their conditions on every push, which is inefficient. The original workflows had proper branch filtering (main/next_major for release, branches-ignore for canary). Consider adding back branch filtering in the trigger, or accept that both jobs will be evaluated on every push (though only one will run due to the if conditions).

Suggested change
push:
push:
branches:
- main
- '**'
branches-ignore:
- changeset-release/main

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +65
- name: Publish canary version
run: |
echo "$( jq '.version = "0.0.0"' package.json )" > package.json
echo -e "---\n'@primer/gatsby-theme-doctocat': patch\n---\n\nFake entry to force publishing" > .changeset/force-snapshot-release.md
rm -f .changeset/pre.json
npx changeset version --snapshot
npx changeset publish --tag canary
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The canary release job is missing NPM authentication setup. The original release_canary.yml workflow (lines 31-37) created an .npmrc file with NPM_TOKEN for authentication. Without this, the 'changeset publish' command on line 65 will fail when attempting to publish to npm.

Copilot uses AI. Check for mistakes.

release-canary:
name: Canary
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && github.ref_name != 'changeset-release/main' }}
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The canary job condition excludes 'changeset-release/main' but the original workflow excluded all 'changeset-release/**' branches (using branches-ignore pattern). This may allow canary releases from other changeset-release branches like 'changeset-release/next_major'. Consider using 'startsWith(github.ref_name, "changeset-release/")' to match the original behavior.

Suggested change
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && github.ref_name != 'changeset-release/main' }}
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && !startsWith(github.ref_name, 'changeset-release/') }}

Copilot uses AI. Check for mistakes.

release-canary:
name: Canary
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && github.ref_name != 'changeset-release/main' }}
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The canary job condition also needs to exclude 'dependabot/**' branches, which were excluded in the original release_canary.yml workflow (line 7 in the old file). Without this exclusion, the workflow will attempt to create canary releases for dependabot branches.

Suggested change
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && github.ref_name != 'changeset-release/main' }}
if: ${{ github.repository == 'primer/doctocat' && github.ref_name != 'main' && github.ref_name != 'changeset-release/main' && !startsWith(github.ref_name, 'dependabot/') }}

Copilot uses AI. Check for mistakes.
@jonrohan jonrohan merged commit a38892c into main Nov 5, 2025
8 checks passed
@jonrohan jonrohan deleted the release_combine branch November 5, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants