-
Notifications
You must be signed in to change notification settings - Fork 2
chore(ts): configures automated version releasing #178
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
Conversation
WalkthroughReplaces the previous semantic-release Bash flow with a release-please driven workflow: adds GitHub Actions jobs (detection, release, publish), removes the Bash release script, adds release-please config and manifest, and updates TypeScript package tooling and npm Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant WF as GitHub Actions
participant HC as has-changes Job
participant RP as release Job (release-please)
participant GH as GitHub API
participant PUB as publish Job
participant NPM as npm Registry
PR->>WF: trigger workflow
WF->>HC: run change detection
HC-->>WF: outputs ts=true/false
alt ts changes detected
WF->>RP: start release job (depends on has-changes)
RP->>GH: create/update release PR via release-please
RP-->>WF: outputs paths_released=["ts"]
WF->>PUB: start publish job (when paths_released contains "ts")
PUB->>PUB: checkout, setup-node (24.12.0), setup-go, vendor install
PUB->>PUB: cd ts/ && npm run release
PUB->>NPM: publish package
PUB-->>WF: publish complete
else no ts changes
WF-->>WF: skip release & publish
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
89a47a0 to
7729ef7
Compare
|
@copilot why release-ts.yml workflow is not executed in this PR? |
This comment was marked as outdated.
This comment was marked as outdated.
7729ef7 to
3c77c77
Compare
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.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @.github/workflows/release-ts.yaml:
- Around line 18-20: Update the GitHub Actions step that uses the
dorny/paths-filter action: replace the usage reference "dorny/paths-filter@v2"
with "dorny/paths-filter@v3" in the workflow step named "Check for changes" (the
step using id "changes") and verify any v3-required input or output name changes
in that step so the workflow remains compatible with the new major version.
- Around line 68-71: Replace the Publish npm package step that currently echoes
the command with one that actually runs the release (change the run from echo
"npm run release" to run: npm run release) and add a prior Configure npm
authentication step that writes the registry token into ts/.npmrc (use the
secrets.NPM_TOKEN) so npm can authenticate before publishing; reference the
workflow job steps named "Configure npm authentication" and "Publish npm
package" to locate and update them.
- Around line 3-6: The workflow currently triggers on pull_request which breaks
release-please and post-merge publishing; change the top-level trigger key from
pull_request to push with branches: [main] so release-please runs on merges to
main and the publish job executes after merge, or if you need PR testing add
both triggers (pull_request and push) and add conditional logic in job-level if
checks (e.g., if: github.event_name == 'push' && github.ref ==
'refs/heads/main') to ensure release-please and the publish job only run on
push-to-main events.
In @ts/package.json:
- Line 102: CI Node version in package.json ("node": ">=22.14.0") and the
workflows are inconsistent with the Volta pin/release workflow; update the test
and lint workflow job configurations to use Node 24.12.0 to match the pinned
development/release version. Locate references to Node versions in test.yaml and
lint.yaml (the jobs that set up Node or use actions/setup-node) and change the
version value from 22.14.0 to 24.12.0; also update the package.json engines
field to "node": ">=24.12.0" if you want the engines constraint to match the
CI/release pin. Ensure all occurrences (including any matrix entries or other
workflow files mentioned in the PR) are updated to 24.12.0 so CI, lint, and
release are aligned.
🧹 Nitpick comments (2)
ts/package.json (1)
43-43: Enhance the release script with provenance and authentication safeguards.The release script runs
npm publishdirectly, which is appropriate for a release-please workflow where version bumping is handled by the bot. However, consider adding the--provenanceflag for supply chain security and ensuring proper npm authentication is configured in the CI environment.🔎 Suggested enhancement
- "release": "npm run build && npm test && npm publish" + "release": "npm run build && npm test && npm publish --provenance --access public"Note: Ensure
NPM_TOKENis configured in the workflow with appropriate permissions for provenance publishing..github/workflows/release-ts.yaml (1)
55-67: Remove unnecessary Go toolchain setup.The Go toolchain setup (lines 57-67) appears unnecessary because:
- No explicit test execution occurs in this workflow
- The
npm run releasescript in package.json already runsnpm test, which should handle all TypeScript testing- This adds ~30-60 seconds of setup time per workflow run
If Go dependencies are genuinely required for TypeScript tests, please clarify the dependency in the comments. Otherwise, consider removing this setup.
🔎 Proposed removal
- name: Setup env uses: HatsuneMiku3939/direnv-action@v1 - - name: Setup go toolchain - run: | - toolchain=$(./script/tools.sh gotoolchain | sed 's/go*//') - echo "GOVERSION=${toolchain}" >> $GITHUB_ENV - - name: Setup go - uses: actions/setup-go@v5 - with: - go-version: "${{ env.GOVERSION }}" - - name: install vendors - # need this to run ts tests - run: make modvendor - name: Publish npm package working-directory: ts
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-ts.yamlscript/release-ts.shts/package.json
💤 Files with no reviewable changes (1)
- script/release-ts.sh
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release-ts.yaml
19-19: the runner of "dorny/paths-filter@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
🔇 Additional comments (1)
.github/workflows/release-ts.yaml (1)
36-39: Clarify: Implementation uses release-please, not release-it as stated in objectives.The PR objectives (issue #149) specify migrating to release-it and replicating the console repository's releaser pattern. However, this workflow implements release-please, which is a different tool with a different workflow model:
- release-it: Interactive release tool that can be run locally or in CI
- release-please: Google's automated release PR creation tool
These tools have different configuration patterns and workflows. Please confirm which tool is intended, or update the PR description to reflect the actual implementation.
c8e73b6 to
03f1c8b
Compare
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.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @.github/workflows/release-ts.yaml:
- Around line 36-40: The release workflow step with id release-pl incorrectly
sets the action input target-branch to the feature branch chore/release-setup;
update the release-please action's target-branch input (the target-branch key
under the step with id release-pl) to point to the repository's main release
branch (e.g., main) so releases are created against the correct branch.
- Around line 25-27: The path filters in the workflow are incorrect because they
omit the .github/ prefix; update the entries 'release-please-config.json',
'workflows/release-ts.yaml', and 'workflows/tests.yaml' to
'.github/release-please-config.json', '.github/workflows/release-ts.yaml', and
'.github/workflows/tests.yaml' respectively in the paths section so changes to
those workflow files are detected and trigger the job.
♻️ Duplicate comments (3)
.github/workflows/release-ts.yaml (3)
3-6: Critical: Workflow trigger still incorrect - this explains why the workflow didn't execute.This issue was previously flagged but remains unresolved. The workflow triggers on
pull_requestbut release-please requirespushto main. This directly answers the question "why release-ts.yml workflow is not executed in this PR?" - because it's configured to run on pull_request events, not on the creation of this PR.Change to:
push: branches: - main
18-20: Update dorny/paths-filter to v3.This issue was previously flagged but remains unresolved. The action version is outdated and may not run correctly on current GitHub Actions runners.
Based on static analysis hints.
69-72: Critical: Publish step still echoes command instead of executing it.This issue was previously flagged but remains unresolved. Line 72 uses
echo "npm run release"which only prints the command without executing it, so no package publishing will occur.🔎 Proposed fix
- name: Publish npm package working-directory: ts run: | - echo "npm run release" + npm run release
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/release-ts.yaml.release-please-manifest.jsonrelease-please-config.jsonscript/release-ts.shts/package.json
💤 Files with no reviewable changes (1)
- script/release-ts.sh
✅ Files skipped from review due to trivial changes (1)
- .release-please-manifest.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ts/package.json
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release-ts.yaml
19-19: the runner of "dorny/paths-filter@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: release
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
🔇 Additional comments (1)
release-please-config.json (1)
1-19: Configuration looks correct for release-please multi-package setup.The release-please configuration correctly implements separate pull requests for reviewable releases, matching the objectives from issue #149. The prerelease versioning with alpha type and tag formatting settings are appropriate for the TypeScript package.
304a402 to
75702cf
Compare
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.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @release-please-config.json:
- Around line 15-16: Add documentation or an inline comment near the
"versioning" and "prerelease-type" settings to record the planned transition
from prerelease to stable; specifically note that currently "versioning":
"prerelease" with "prerelease-type": "alpha" produces alpha prereleases, and
that to publish stable versions you will change "versioning" to "default" and
create a release PR to transition to stable (e.g., 1.0.0). Include the intended
timing or trigger for the change (milestone, date, or condition) so future
maintainers know when to flip "versioning" from "prerelease" to "default".
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-ts.yaml (1)
21-24: Incomplete path filters will miss release configuration changes.The filters only include
ts/**, but changes to release-please configuration files and this workflow itself won't trigger the release process. This means updates to versioning strategy, changelog format, or the workflow logic would be silently ignored.🔎 Proposed fix
filters: | ts: - 'ts/**' + - 'release-please-config.json' + - '.release-please-manifest.json' + - '.github/workflows/release-ts.yaml'
♻️ Duplicate comments (1)
.github/workflows/release-ts.yaml (1)
18-18: Outdated action version persists.The
dorny/paths-filter@v2action remains outdated despite the previous review comment. GitHub Actions runners may not support this version correctly.🔎 Proposed fix
- uses: dorny/paths-filter@v2 + uses: dorny/paths-filter@v3Based on static analysis hints.
🧹 Nitpick comments (1)
.github/workflows/release-ts.yaml (1)
40-40: Brittle string comparison for paths_released.The condition uses string equality
== '["ts"]'to check a JSON array, which is fragile and may break if release-please changes output formatting or if multiple paths are released.🔎 More robust approach
- if: ${{ needs.release.outputs.paths_released == '["ts"]' }} + if: ${{ contains(fromJSON(needs.release.outputs.paths_released || '[]'), 'ts') }}This handles:
- Multiple packages released simultaneously
- Empty/null outputs (default to empty array)
- Future format changes
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/release-ts.yaml.release-please-manifest.jsonrelease-please-config.jsonscript/release-ts.shts/package.json
💤 Files with no reviewable changes (1)
- script/release-ts.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .release-please-manifest.json
- ts/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: go
🔇 Additional comments (2)
.github/workflows/release-ts.yaml (2)
64-66: The release script is incomplete and doesn't actually attempt to publish.The
ts/package.jsonrelease script exits with code 1 after just echoing 'npm publish':"release": "npm run build && npm test && echo 'npm publish' && exit 1"This is a stub that fails intentionally. The workflow job will fail on this step, so the missing npm authentication is not the current issue. Either implement the actual npm publish command in the release script or update the workflow step to run
npm publishdirectly with proper authentication.Likely an incorrect or invalid review comment.
53-63: The original review comment is based on incorrect assumptions about the workflow architecture.The
npm run releasecommand includesnpm testas part of its execution, so tests are already running in the publish job—not being skipped. The Go toolchain setup andmake modvendorare necessary dependencies, not overhead:modvendorvendors non-Go files (including.protofiles) from Go dependencies, which are required for TypeScript proto generation. The build entrypoints inesbuild.config.mjsinclude'src/generated/protos/index.*', confirming that the generated proto files are essential build artifacts.The architecture is correct: Go module vendoring → proto generation → TypeScript build with generated protos → tests. Moving tests to a separate job earlier in the pipeline would not be beneficial, as they legitimately depend on the proto generation step that occurs during the build phase.
Likely an incorrect or invalid review comment.
|
example PR -> #180 |
75702cf to
91016f1
Compare
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-ts.yaml (1)
12-24:has-changesjob requiresactions/checkoutto detect changes on push eventsThe
dorny/paths-filteraction usesgitto compute changed paths onpushevents (viamerge-baseor commit history comparison); without a checked-out repository, change detection will fail or produce incorrect results.Proposed fix
has-changes: runs-on: ubuntu-latest outputs: ts: ${{ steps.changes.outputs.ts }} steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 2 - name: Check for changes uses: dorny/paths-filter@v2Consider also upgrading
dorny/paths-filterfrom v2 to v3 (latest stable release).
🤖 Fix all issues with AI agents
In @.github/workflows/release-ts.yaml:
- Around line 25-36: Add explicit write permissions required by
googleapis/release-please-action: update the workflow to grant permissions for
contents: write, issues: write, and pull-requests: write (either at the workflow
top-level or inside the release job) so the step using release-please-action
(id: release-pl) can create/update release PRs and tags when using the provided
token (${{ secrets.GITHUB_TOKEN }}).
- Around line 37-66: The publish job needs two fixes: add an explicit dependency
install step before the "Publish npm package" step by running npm ci in the ts
working-directory, and make the job if-condition robust by replacing if: ${{
needs.release.outputs.paths_released == '["ts"]' }} with if: ${{
contains(fromJSON(needs.release.outputs.paths_released), 'ts') }} so it handles
different output formats; locate the publish job and the "Publish npm package"
step to insert the npm ci step immediately before the npm run release invocation
and update the job-level if expression accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/release-ts.yaml.release-please-manifest.jsonrelease-please-config.jsonscript/release-ts.shts/package.json
💤 Files with no reviewable changes (1)
- script/release-ts.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- .release-please-manifest.json
- release-please-config.json
- ts/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: proto
- GitHub Check: go
91016f1 to
33abaad
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/release-ts.yaml (2)
12-23: Consider adding checkout step for reliability.While
dorny/paths-filter@v2can work without checkout for push events by using the GitHub API, adding an explicit checkout step would improve reliability and performance.💡 Suggested enhancement
has-changes: runs-on: ubuntu-latest outputs: ts: ${{ steps.changes.outputs.ts }} steps: + - uses: actions/checkout@v6 - name: Check for changes uses: dorny/paths-filter@v2 id: changes with: filters: | ts: - 'ts/**'
25-40: Add checkout step for release-please-action.The
googleapis/release-please-action@v4needs to read configuration files (.release-please-manifest.json,release-please-config.json) from the repository. While the action can clone the repository internally using the GitHub token, adding an explicit checkout step is the recommended approach for reliability and performance.🔧 Recommended fix
release: runs-on: ubuntu-latest needs: has-changes outputs: paths_released: ${{ steps.release-pl.outputs.paths_released }} if: needs.has-changes.outputs.ts == 'true' permissions: contents: write issues: write pull-requests: write steps: + - uses: actions/checkout@v6 - uses: googleapis/release-please-action@v4 id: release-pl with: token: ${{ secrets.GITHUB_TOKEN }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release-ts.yaml.release-please-manifest.jsonmake/release-ts.mkrelease-please-config.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .release-please-manifest.json
- release-please-config.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (6)
make/release-ts.mk (2)
3-3: LGTM: Clean delegation to npm scripts.The refactoring to use
npm run releasealigns well with the new release-please workflow and improves maintainability by centralizing release logic in package.json.
6-6: LGTM: Build target delegation.The build target correctly delegates to the npm build script.
.github/workflows/release-ts.yaml (4)
57-67: Go toolchain setup is appropriate.While unusual for a TypeScript/npm package release, the Go toolchain setup is correctly explained by the inline comment indicating it's needed to run TypeScript tests. The implementation correctly derives the Go version and sets up the environment.
49-69: Overall publish job structure looks sound.The publish job correctly:
- Depends on the release job
- Runs conditionally when a release is created
- Sets up the complete environment (Node.js, direnv, Go)
- Executes the release via make
The workflow aligns with the PR objective of using release-please to create reviewable release PRs.
50-52: No issue. Node.js version 24.12.0 is a valid LTS release (released December 10, 2025) and is available on the official Node.js website.Likely an incorrect or invalid review comment.
41-48: The conditional logic is correct.The
paths_releasedoutput fromgoogleapis/release-please-action@v4is indeed a JSON-encoded string containing an array of paths (e.g.,"[]"or'[".", "packages/my-lib"]'). UsingfromJSON()to parse it andcontains()to check for'ts'is the proper way to evaluate this condition.
📝 Description
configures automated version releasing based on https://github.com/googleapis/release-please-action and https://github.com/googleapis/release-please
It aggregates releases via release PR:
Publishing is disabled for now, to ensure it works as expected first
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist
📎 Notes for Reviewers
[Include any additional context, architectural decisions, or specific areas to focus on]