Conversation
|
📝 WalkthroughWalkthroughAdds the Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 232ee41
☁️ Nx Cloud last updated this comment at |
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (14.77%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #551 +/- ##
===========================================
- Coverage 70.90% 14.77% -56.14%
===========================================
Files 53 153 +100
Lines 2021 26262 +24241
Branches 377 1056 +679
===========================================
+ Hits 1433 3879 +2446
- Misses 588 22383 +21795 🚀 New features to boost your workflow:
|
|
Deployed 0c3b703 to https://ForgeRock.github.io/ping-javascript-sdk/pr-551/0c3b703af7bced717105c81cc5f22ca2325f19f2 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🆕 New Packages🆕 @forgerock/journey-client - 87.8 KB (new) ➖ No Changes➖ @forgerock/sdk-logger - 1.6 KB 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
18-18: Consider consolidating release builds to a single stage.With Line 18 building in
ci:release, and publish actions also building (.github/actions/publish-release/action.ymlLine 35,.github/actions/publish-beta/action.ymlLine 13), release flows can perform redundant non-cached builds. Consider keeping one authoritative build step to reduce publish latency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 18, The CI currently runs builds in the npm script "ci:release" and again inside the publish actions, causing duplicate non-cached builds; pick one authoritative build step (recommend keeping "ci:release" as the single build stage) and remove or gate the build steps in the publish actions (.github/actions/publish-release/action.yml and .github/actions/publish-beta/action.yml) so they don’t rebuild artifacts—either remove the build command from those action.yml files or add an input/flag (e.g., skip-build) to short-circuit the build when artifacts are produced by "ci:release", and ensure the publish actions consume the produced build artifacts instead.
🤖 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/publish.yml:
- Line 99: The current job guard only checks github.ref and can be bypassed when
workflow_dispatch supplies a user-controlled inputs.branch; update the job's
if-condition to also validate the dispatched input by adding checks against
github.event.inputs.branch (or github.event.inputs.branch prefixed with
refs/heads/) so it does not equal the protected release branch (e.g.,
"changeset-release/main" and "refs/heads/changeset-release/main"); ensure the
expression used for the job run condition combines github.event_name ==
'workflow_dispatch', github.ref != 'refs/heads/changeset-release/main', and the
new github.event.inputs.branch != 'changeset-release/main' /
github.event.inputs.branch != 'refs/heads/changeset-release/main' checks so the
snapshot publishing step that reads inputs.branch cannot be pointed at the
protected branch.
---
Nitpick comments:
In `@package.json`:
- Line 18: The CI currently runs builds in the npm script "ci:release" and again
inside the publish actions, causing duplicate non-cached builds; pick one
authoritative build step (recommend keeping "ci:release" as the single build
stage) and remove or gate the build steps in the publish actions
(.github/actions/publish-release/action.yml and
.github/actions/publish-beta/action.yml) so they don’t rebuild artifacts—either
remove the build command from those action.yml files or add an input/flag (e.g.,
skip-build) to short-circuit the build when artifacts are produced by
"ci:release", and ensure the publish actions consume the produced build
artifacts instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9f36b9b-9604-4cd8-b8b9-9f87be4f224d
📒 Files selected for processing (4)
.github/actions/publish-beta/action.yml.github/actions/publish-release/action.yml.github/workflows/publish.ymlpackage.json
ryanbas21
force-pushed
the
update-publish-steps
branch
from
6272a18 to
232ee41
Compare
3 participants
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-4858
Description
Due to unforeseen issues, we want to guard the job against the changesets release branch.
Additionally add steps to force no agents (no distributed build tasks) and no nx cache when running build in publish steps
Summary by CodeRabbit