chore: adds version update during docker build#2784
Conversation
📝 WalkthroughWalkthroughUpgrades the reusable build workflow (checkout v4→v6); adds APP_PATH and APP_VERSION env vars and a Node.js one-liner to update package version fields before invoking the build script (with quoted args); removes an automated PR-approval workflow and adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/reusable-build-image.yml:
- Around line 133-137: The npm version step is using the raw inputs.tag ("${{
inputs.tag }}") which contains prefixes like "console-api/v1.2.3" and is not
valid semver; update the npm version invocation to use the cleaned semver stored
in env.tag (replace references to inputs.tag with env.tag) so the
--workspace="apps/${{ env.app }}" npm version command receives a valid version
string generated earlier (see the env.tag setup around lines 116-118).
e77456c to
1901f4e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.gitignore (1)
101-101: Consider a more specific pattern or add documentation.The broad pattern
.github/workflows/test*.ymlwill ignore all workflow files starting with "test", which could accidentally prevent legitimate test workflows (e.g.,test-integration.yml,test-e2e.yml) from being committed.Consider either:
- Using a more specific pattern if only temporary/local test files should be ignored (e.g.,
.github/workflows/test-local*.yml)- Adding an inline comment explaining why test workflows should be ignored
📝 Proposed improvement with documentation
.claude/settings.local.json +# Ignore temporary test workflow files (use test-local* prefix for experiments) .github/workflows/test*.ymlOr with a more specific pattern:
.claude/settings.local.json -.github/workflows/test*.yml +# Ignore local/temporary test workflow files +.github/workflows/test-local*.yml +.github/workflows/test-temp*.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 101, The .gitignore entry `.github/workflows/test*.yml` is too broad and may hide legitimate workflows; update this by either narrowing the glob to the intended files (e.g., change the pattern to `.github/workflows/test-local*.yml` or another precise prefix) or keep the existing pattern but add an inline comment above it explaining exactly which temporary/local test workflow files should be ignored and why so reviewers understand the intent; locate the current entry `.github/workflows/test*.yml` and apply one of these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.gitignore:
- Line 101: The .gitignore entry `.github/workflows/test*.yml` is too broad and
may hide legitimate workflows; update this by either narrowing the glob to the
intended files (e.g., change the pattern to `.github/workflows/test-local*.yml`
or another precise prefix) or keep the existing pattern but add an inline
comment above it explaining exactly which temporary/local test workflow files
should be ignored and why so reviewers understand the intent; locate the current
entry `.github/workflows/test*.yml` and apply one of these changes.
7d0d04d to
9aedc0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/reusable-build-image.yml:
- Around line 135-143: Replace the single-quoted node one-liner that triggers
ShellCheck SC2016 (the node -e '...' block referencing process.env.APP_PATH and
process.env.APP_VERSION and reading/writing package-lock.json and package.json)
with a heredoc that feeds the JavaScript to node (e.g., use node <<'NODE' ...
NODE or cat <<'NODE' | node with a quoted delimiter) so the script is not
wrapped in single quotes and shellcheck/actionlint stop flagging SC2016; keep
the same logic (reading package-lock.json and
./${process.env.APP_PATH}/package.json, updating versions, and writing files)
but place it inside the heredoc block.
Why
To let apps work and expose version as they did before
What
Summary by CodeRabbit