ci: consolidate and streamline workflows #1093
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a new composite GitHub Action for Node setup/build, replaces and refactors deployment and PR workflows to use it, introduces a comprehensive PR validation/preview workflow, removes several legacy workflows, adds a project-level .npmrc, and applies minor style tweaks to scorecard.yml. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant GH as GitHub
participant PRWF as .github/workflows/pr.yml
participant SA as .github/actions/setup-and-build
participant Art as Artifact Store
participant GP as gh-pages
Dev->>GH: Open/update PR
GH->>PRWF: Trigger PR workflow
PRWF->>SA: code-checks (setup=true, build=false)
SA-->>PRWF: checks complete
PRWF->>SA: build-test (setup=true, build=true, BASE_URL=/pr-preview/pr-<num>)
SA-->>PRWF: build artifacts
PRWF->>Art: upload build artifacts
PRWF->>GP: deploy-preview (sync pr-preview/pr-<num>)
PRWF-->>GH: post preview URL comment
GH->>PRWF: PR closed
PRWF->>GP: cleanup-preview (remove pr-preview/pr-<num>)
sequenceDiagram
participant WF as Calling Workflow
participant ACT as setup-and-build (composite)
WF->>ACT: inputs (node-version, setup, build, build-analytics-id, build-base-url)
alt inputs.setup == 'true'
ACT->>actions/setup-node: setup Node.js (pinned)
ACT->>shell: npm ci
end
alt inputs.build == 'true'
ACT->>shell: npm run build (env: HUBSPOT_TRACKING_ID, BASE_URL)
end
ACT-->>WF: exit/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
evansims
force-pushed
the
ci/streamline-workflows
branch
8 times, most recently
from
9cd2e2e to
464fb90
Compare
![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 (2)
.github/actions/setup-and-build/action.yml (1)
41-48: Consider caching keyed to lockfiles for better cache hitsIf this repo ever uses nested packages or the lockfile moves, you’ll get more stable cache keys by specifying dependency paths.
Apply this diff:
- name: Setup Node.js if: inputs.setup == 'true' uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: ${{ inputs.node-version }} registry-url: "https://registry.npmjs.org" - cache: "npm" + cache: "npm" + # Adjust globs if you add workspaces or move lockfiles + cache-dependency-path: | + package-lock.json.github/workflows/pr.yml (1)
84-89: Simplify condition: remove unnecessary always()The needs guard already controls execution. always() here is redundant.
Apply this diff:
- if: | - always() && - github.event.action != 'closed' && + if: | + github.event.action != 'closed' && github.event.pull_request.draft == false && (needs.code-checks.result == 'success' || needs.code-checks.result == 'skipped')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.github/actions/setup-and-build/action.yml(1 hunks).github/workflows/checks.yaml(0 hunks).github/workflows/deploy.yml(1 hunks).github/workflows/pr.yml(1 hunks).github/workflows/preview.yml(0 hunks).github/workflows/scorecard.yml(4 hunks).github/workflows/test-deploy.yml(0 hunks).npmrc(1 hunks)
💤 Files with no reviewable changes (3)
- .github/workflows/preview.yml
- .github/workflows/test-deploy.yml
- .github/workflows/checks.yaml
🔇 Additional comments (5)
.github/workflows/scorecard.yml (1)
15-15: Style-only changes look goodSwitching to single quotes and compact branch array has no functional impact. Workflow remains equivalent.
Also applies to: 36-41, 66-76
.github/actions/setup-and-build/action.yml (1)
33-48: Checkout/Node setup composition looks solid
- LFS-cached checkout and standard checkout are correctly gated.
- Node is pinned via actions/setup-node with npm cache enabled and registry set.
.github/workflows/deploy.yml (1)
21-25: Good consolidation via local composite actionUsing the reusable action simplifies the deploy flow and keeps versions pinned. Passing hubspot-tracking-id via inputs is clean.
.github/workflows/pr.yml (2)
83-101: Confirm intended behavior for Dependabot PRs in build-testCurrently, Dependabot PRs still run the full build (only artifact upload is skipped). If the goal is to avoid unnecessary compute for Dependabot, gate the entire job with an additional actor check.
If you want to skip building for Dependabot entirely, apply:
name: Build and Test - if: | - github.event.action != 'closed' && - github.event.pull_request.draft == false && - (needs.code-checks.result == 'success' || needs.code-checks.result == 'skipped') + if: | + github.event.action != 'closed' && + github.actor != 'dependabot[bot]' && + github.event.pull_request.draft == false && + (needs.code-checks.result == 'success' || needs.code-checks.result == 'skipped')Would you like me to propagate the same pattern to other jobs as needed?
134-149: Step outputs reference will not resolve with bracket syntax; use a hyphen-free idGitHub Actions expressions don’t support bracket indexing for step outputs. With an id that contains a hyphen, dot-notation can be misparsed. Rename the step id to use underscores and reference with dot-notation.
Apply this diff:
- - name: Deploy preview - uses: rossjrw/pr-preview-action@9f77b1d057b494e662c50b8ca40ecc63f21e0887 # v1.6.2 - id: preview-step + - name: Deploy preview + uses: rossjrw/pr-preview-action@9f77b1d057b494e662c50b8ca40ecc63f21e0887 # v1.6.2 + id: preview_step with: source-dir: ./build/ preview-branch: gh-pages umbrella-dir: pr-preview action: deploy - - name: Publish preview link - if: steps['preview-step'].outputs['deployment-action'] == 'deploy' + - name: Publish preview link + if: steps.preview_step.outputs.deployment-action == 'deploy' run: | - url="${{ steps['preview-step'].outputs['preview-url'] }}" + url="${{ steps.preview_step.outputs.preview-url }}" echo "Preview visible at ${url}" >> "$GITHUB_STEP_SUMMARY" echo "[Open preview](${url})" >> "$GITHUB_STEP_SUMMARY"Likely an incorrect or invalid review comment.
evansims
force-pushed
the
ci/streamline-workflows
branch
2 times, most recently
from
c330578 to
61a9a6e
Compare
evansims
force-pushed
the
ci/streamline-workflows
branch
from
61a9a6e to
d072912
Compare
|
evansims
force-pushed
the
ci/streamline-workflows
branch
from
d072912 to
fd9f0d4
Compare
Preview DeploymentPreview is ready at: https://openfga.github.io/openfga.dev/pr-preview/pr-1093/ This preview will be automatically removed when the PR is closed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
.github/workflows/scorecard.yml (1)
25-26: Minor: Remove dead condition referencing pull_request.This workflow does not trigger on pull_request events, so the
|| github.event_name == 'pull_request'portion in the job-level if is effectively dead code. Consider removing for clarity.- if: github.event.repository.default_branch == github.ref_name || github.event_name == 'pull_request' + if: github.event.repository.default_branch == github.ref_name.github/actions/setup-and-build/action.yml (2)
37-41: Optional: allow passing npm ci flags (security/perf toggles).Some workflows prefer
npm ciwith flags like--ignore-scripts,--no-audit, or--no-fundfor supply-chain hardening or speed. Making this configurable keeps the action reusable without hardcoding a policy.inputs: node-version: @@ build-base-url: description: "Base URL for build" required: false default: "" + npm-ci-flags: + description: "Additional flags for npm ci (e.g. --ignore-scripts --no-audit --no-fund)" + required: false + default: "" @@ - name: Install dependencies if: inputs.setup == 'true' shell: bash - run: npm ci + run: npm ci ${{ inputs.npm-ci-flags }}
42-48: Optional: support custom build command/dir for reusability.If you foresee building from subdirectories or with non-standard build scripts, consider inputs like
workdirandbuild-commandto avoid duplicating steps in downstream workflows..github/workflows/pr.yml (4)
27-43: Consider pinning the madge version to avoid drift.Running
npx madgewithout a pinned version can introduce nondeterminism across CI runs. Prefer adding madge as a devDependency and invoking via an npm script (or pinning vianpx -y madge@<version>).
73-86: Explicitly set the folder path for markdown link check.To avoid relying on action defaults and ensure both .md and .mdx files under repo root are scanned, pass
folder-path: '.'.- name: Check Markdown links (${{ matrix.extension }}) uses: tcort/github-action-markdown-link-check@a800ad5f1c35bf61987946fd31c15726a1c9f2ba # v1.1.0 continue-on-error: true with: file-extension: ${{ matrix.extension }} use-quiet-mode: 'yes' - config-file: '.github/workflows/markdown.links.config.json' + config-file: '.github/workflows/markdown.links.config.json' + folder-path: '.'
218-227: Match comments by author login to avoid updating other bots.Filtering by
comment.user.type === 'Bot'could catch unrelated bot comments. Prefer matching the GitHub Actions bot login explicitly.- const botComment = comments.find(comment => - comment.user.type === 'Bot' && - comment.body.includes('Preview Deployment') - ); + const botComment = comments.find(comment => + comment.user?.login === 'github-actions[bot]' && + comment.body?.includes('Preview Deployment') + );
279-306: Make preview cleanup resilient for untracked dirs.If the preview dir somehow becomes untracked,
git rm -rfwill fail. You can remove it withrm -rf, then stage deletions withgit add -A, which works whether files are tracked or not.- if [ -d "${preview_dir}" ]; then - git rm -rf "${preview_dir}" - git commit -m "Remove preview for PR #${{ github.event.pull_request.number }} 🧹" + if [ -d "${preview_dir}" ]; then + rm -rf "${preview_dir}" + git add -A + if git diff --staged --quiet; then + echo "⚠️ Preview directory existed but no tracked files to remove" + else + git commit -m "Remove preview for PR #${{ github.event.pull_request.number }} 🧹" + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.github/actions/setup-and-build/action.yml(1 hunks).github/workflows/checks.yaml(0 hunks).github/workflows/deploy.yml(1 hunks).github/workflows/pr.yml(1 hunks).github/workflows/preview.yml(0 hunks).github/workflows/scorecard.yml(4 hunks).github/workflows/test-deploy.yml(0 hunks).npmrc(1 hunks)
💤 Files with no reviewable changes (3)
- .github/workflows/preview.yml
- .github/workflows/checks.yaml
- .github/workflows/test-deploy.yml
✅ Files skipped from review due to trivial changes (1)
- .npmrc
🔇 Additional comments (9)
.github/workflows/scorecard.yml (2)
15-15: LGTM on formatting tweak.Switching to single-quoted YAML scalars is fine and keeps things consistent. No functional change.
36-79: Good security posture: actions pinned and perms scoped.Steps are pinned by SHA and permissions are minimal, which is great for supply-chain hardening.
.github/actions/setup-and-build/action.yml (1)
29-36: Action pinning and npm cache usage look solid.Using actions/setup-node pinned by SHA with npm caching is a good baseline for speed and reproducibility.
.github/workflows/deploy.yml (3)
9-17: Scoped permissions and concurrency policy are appropriate.Minimal perms with pages/id-token and a single concurrency group for pages reduce risk and avoid overlapping deploys.
23-44: Build pipeline is clean and uses the new composite action effectively.Checkout (with LFS), centralized setup/build, CNAME creation, configure-pages, and artifact upload are wired correctly with pinned SHAs.
45-56: Deploy step aligns with official Pages actions.Using actions/deploy-pages with the output URL plumbed to the environment is the recommended pattern.
.github/workflows/pr.yml (3)
18-21: Concurrency key is well-chosen.Using PR number or ref avoids parallel runs per PR and cancels in-flight builds on updates.
134-146: Ensure gh-pages exists for preview deployments.Checkout will fail if gh-pages doesn’t exist. This repo likely has it, but if you ever reset, consider guarding or creating it on-demand.
247-254: Nice UX touch with step summary.Including a direct link in the job summary is helpful for reviewers.
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates and streamlines the repository's CI workflows to improve efficiency and reduce redundant builds. The main changes include creating a reusable composite action, merging multiple workflow files into a single comprehensive PR workflow, and updating deployment workflows to use official GitHub Pages actions.
- Consolidates checks, preview, and test deployment workflows into a single PR workflow with better dependency management
- Creates a reusable composite action for common setup and build steps
- Modernizes deployment workflows by replacing third-party actions with official GitHub Pages actions
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.npmrc |
Adds npm registry configuration to ensure consistent package installations |
.github/workflows/test-deploy.yml |
Removes legacy test deployment workflow (functionality moved to pr.yml) |
.github/workflows/scorecard.yml |
Updates formatting and removes unnecessary comments for consistency |
.github/workflows/preview.yml |
Removes legacy preview workflow (functionality moved to pr.yml) |
.github/workflows/pr.yml |
New comprehensive PR workflow consolidating code checks, build testing, preview deployment, and cleanup |
.github/workflows/deploy.yml |
Updates to use official GitHub Pages actions and removes nightly scheduled deployment |
.github/workflows/checks.yaml |
Removes legacy checks workflow (functionality moved to pr.yml) |
.github/actions/setup-and-build/action.yml |
New composite action for reusable setup and build steps |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
evansims
force-pushed
the
ci/streamline-workflows
branch
2 times, most recently
from
a7f7ef1 to
6c20cdb
Compare
evansims
force-pushed
the
ci/streamline-workflows
branch
from
6c20cdb to
20c339d
Compare
| @@ -1,54 +1,79 @@ | |||
| name: Deploy to GitHub Pages | |||
|
|
|||
| on: | |||
| # Nightly @ 5AM UTC | |||
| schedule: | |||
| - cron: '0 5 * * *' | |||
There was a problem hiding this comment.
I can't think of any reason why this needs to run on a schedule; I can re-add if I've overlooked something
Description
This pull request streamlines our repository CI workflows to speed up CI processes, avoid redundant builds across workflows, and support failing fast as appropriate.
setup-and-buildfor the common setup and build steps shared across various workflowschecks.yaml,preview.yaml, andtest-deploy.yamlinto a consolidated workflowscorecardfor formatting consistencypeaceiris/actions-gh-pagesaction with the official GitHub Pages actionsrossjrw/pr-preview-actionwith DIY solutionI've also added an
.npmrcto the root of the repository to avoid accidental inclusion of assets from other registries in our lockfilesReferences
Review Checklist
mainSummary by CodeRabbit