-
Notifications
You must be signed in to change notification settings - Fork 3
ci: move-to-one-file #420
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
ci: move-to-one-file #420
Conversation
|
WalkthroughReworked .github/workflows/publish.yml to run via workflow_dispatch, refactored the snapshot/trusted-publish job into a single “Publish Snapshots” job, added env vars and npm registry configuration, and wired branch selection via inputs. Removed .github/workflows/snapshot.yml that previously called the publish workflow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer (manual)
participant GH as GitHub Actions
participant Repo as Repo
participant NX as Nx Tasks
participant NPM as npm Registry
Dev->>GH: Run "Publish Snapshots"<br/>(workflow_dispatch with inputs.branch, dist_tag, prerelease, access)
GH->>Repo: actions/checkout (ref = inputs.branch)
GH->>GH: Setup Node (registry-url=https://registry.npmjs.org)
GH->>GH: Set env (NX_CLOUD_DISTRIBUTED_EXECUTION, PNPM_CACHE_FOLDER, CODECOV_TOKEN, CI)
GH->>NX: nx-set-shas(main-branch-name=main)
GH->>NX: Build / Test / E2E CI
alt prerelease flow
GH->>Repo: Version packages as prerelease (inputs.prerelease)
GH->>NPM: Publish packages (dist-tag=inputs.dist_tag, access=inputs.access)
end
GH-->>Dev: Job complete (artifacts/logs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
|
View your CI Pipeline Execution ↗ for commit cec2065
☁️ Nx Cloud last updated this comment at |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/publish.yml (2)
178-181: Set registry-url in snapshot job too (consistency + npm auth wiring)Without
registry-url, npm may not write the expected .npmrc config; add it to mirror the push job.Apply this diff:
- uses: actions/setup-node@v4 with: node-version-file: '.node-version' cache: 'pnpm' + registry-url: 'https://registry.npmjs.org'
213-221: Add provenance and update lockfile before snapshot publish
- After
pnpm changeset version --snapshotrunpnpm install --no-frozen-lockfileso the lockfile matches versioned packages (edit .github/workflows/publish.yml near the "Version Packages as prerelease" step).- Enable provenance on publish: add
--provenancetopnpm publish(or setNPM_CONFIG_PROVENANCE: true/npm_config_provenance) and ensure the job hasid-token: writeso OIDC-based provenance can be minted (update publish steps in .github/workflows/publish.yml and any scripts that callpnpm publish, e.g.ci:release).Apply this diff:
- name: Version Packages as prerelease run: pnpm changeset version --snapshot ${{ inputs.prerelease }} env: GITHUB_TOKEN: ${{ secrets.GH_TOKEN }} + - name: Update lockfile after versioning + run: pnpm install --no-frozen-lockfile + # The actual npm publish that must occur in the authorized file - name: Publish packages with dist-tag - run: pnpm publish -r --tag ${{ inputs.dist_tag }} --no-git-checks --access ${{ inputs.access }} + run: pnpm publish -r --tag ${{ inputs.dist_tag }} --no-git-checks --access ${{ inputs.access }} --provenance
🧹 Nitpick comments (4)
.github/workflows/publish.yml (4)
31-34: Scope CODECOV_TOKEN to steps instead of top‑level envMinimize secret exposure. Pass CODECOV_TOKEN only to steps that need it (nx-cloud start-ci-run and codecov) rather than in the global
env.Example (outside this hunk):
- Remove CODECOV_TOKEN from the top-level
env.- Add
env: { CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} }to the two nx-cloud steps and the codecov step.
158-166: Tighten job permissions for snapshot path
issues/pull-requestswrite aren’t used here. Keep least-privilege:contents: read(or omit),id-token: write.Apply this diff:
- permissions: - contents: write - id-token: write - issues: write - pull-requests: write + permissions: + contents: read + id-token: write
62-62: Pass CODECOV_TOKEN to nx-cloud step via env (if you scope it down)If you move CODECOV_TOKEN out of the global env, add it here:
Example (outside this hunk):
- run: pnpm dlx nx-cloud start-ci-run --distribute-on=".nx/workflows/dynamic-changesets.yml" --stop-agents-after="e2e-ci" --with-env-vars="CODECOV_TOKEN" env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}Repeat for the snapshot job’s analogous step.
213-217: Optional: include provenance in the push (changesets) path as wellEnsure
pnpm ci:releasepublishes with provenance too; if not, addNPM_CONFIG_PROVENANCE: trueto the changesets step env.Example (outside this hunk):
- name: publish uses: changesets/action@v1 env: HOME: ${{ github.workspace }} GITHUB_TOKEN: ${{ secrets.GH_TOKEN }} NPM_CONFIG_PROVENANCE: trueTo verify, search your scripts for
--provenanceor an.npmrcwithprovenance=true(the script above checks this).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish.yml(3 hunks).github/workflows/snapshot.yml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/snapshot.yml
⏰ 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). (1)
- GitHub Check: pr
🔇 Additional comments (4)
.github/workflows/publish.yml (4)
6-12: Manual dispatch + branch input wiring: LGTMSingle entry workflow with
workflow_dispatchand checkoutref: ${{ inputs.branch }}matches the “one file” trusted‑publisher requirement.
17-27: Explicit input types: LGTMAdding
type: stringkeeps inputs explicit and future‑proof.
58-58: Registry configured: LGTM
registry-url: https://registry.npmjs.orgis correct for publish provenance/OIDC.
196-199: nx-set-shas main branch hint: LGTMSetting
main-branch-name: mainis correct for affected calculations on dispatch.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #420 +/- ##
=======================================
Coverage 56.19% 56.19%
=======================================
Files 32 32
Lines 2091 2091
Branches 353 353
=======================================
Hits 1175 1175
Misses 916 916 🚀 New features to boost your workflow:
|
|
Deployed 85bdb75 to https://ForgeRock.github.io/ping-javascript-sdk/pr-420/85bdb75a1e947a992a58ce01b3c2d3e2ca29694d branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis➖ No Changes➖ @forgerock/davinci-client - 34.5 KB 11 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
JIRA Ticket
n/a
Description
Final time i think! can't be two files, it will use the original file which breaks auth!
Summary by CodeRabbit