-
Notifications
You must be signed in to change notification settings - Fork 5
chore: add build workflow #416
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
WalkthroughA GitHub Actions CI workflow is added to build all packages on pull requests and pushes to dev/main branches. In parallel, a React component for displaying user follow statistics is refactored to use explicit function logic with improved numeric type safety. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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
🧹 Nitpick comments (2)
.github/workflows/build.yml (1)
10-12: Consider adding a timeout to prevent runaway jobs.Add a timeout to the job definition to prevent excessive CI minutes consumption:
jobs: build: runs-on: ubuntu-latest + timeout-minutes: 30 steps:Adjust the value based on typical build duration for your monorepo.
platforms/blabsy/src/components/user/user-follow-stats.tsx (1)
52-65: UsestatsValueconsistently throughout the render logic.Line 53 introduces a defensive type guard for
statsValue, but line 60 still renders the originalstatsvariable. For consistency and type safety, usestatsValuein both locations.Apply this diff:
].map(([title, stats], index) => { const statsValue = typeof stats === 'number' ? stats : 0; return ( <div key={title} className='mt-0.5 mb-[3px] flex h-4 items-center gap-1 pointer-events-none' > <p className='font-bold text-light-primary dark:text-dark-primary'> - {stats} + {statsValue} </p> <p>{index === 1 && statsValue > 1 ? `${title}s` : title}</p> </div> ); })}This ensures consistent handling of the potentially non-numeric value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks)platforms/blabsy/src/components/user/user-follow-stats.tsx(1 hunks)
⏰ 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: lint
🔇 Additional comments (1)
.github/workflows/build.yml (1)
32-37: Good defensive handling of native modules.The approach of cleaning pre-built binaries and rebuilding is solid for ensuring compatibility across different environments.
| - name: Install pnpm | ||
| run: npm install -g pnpm | ||
|
|
||
| - name: Install Dependencies | ||
| run: pnpm install |
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.
🛠️ Refactor suggestion | 🟠 Major
Use pnpm/action-setup and enable dependency caching.
Installing pnpm via npm doesn't pin the version and misses out on built-in caching. The official pnpm/action-setup action provides version pinning from package.json and integrates with GitHub Actions cache.
Apply this diff:
- - name: Install pnpm
- run: npm install -g pnpm
+ - name: Install pnpm
+ uses: pnpm/action-setup@v4
+ with:
+ run_install: false
- name: Install Dependencies
run: pnpm installAdditionally, add caching for dependencies between the Node.js setup and pnpm install:
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 22.x
cache: 'pnpm'This will significantly reduce CI execution time on subsequent runs.
🤖 Prompt for AI Agents
.github/workflows/build.yml around lines 26 to 30: the workflow installs pnpm
globally via npm and runs pnpm install which neither pins pnpm version nor uses
Actions' dependency caching; replace the two steps with the official
pnpm/action-setup action (configured to read the pnpm version from package.json
or a pinned version) and ensure caching by using actions/setup-node@v4 with
with: node-version: 22.x and cache: 'pnpm' before running pnpm install so
dependencies are cached between runs.
Description of change
add a build check on CI
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit