Add CI checks for web workspace #11
Conversation
|
@duck-master is attempting to deploy a commit to the polylove Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a CI workflow for the web workspace and applies small formatting, prop-usage, and UI/state-commenting changes across multiple web components; no functional control-flow changes are introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Repo as web workspace
Dev->>GH: Push/PR to main with web/**/* changes
GH->>Repo: Checkout & setup node, install deps
GH->>Repo: Run Prettier check
GH->>Repo: Run ESLint
GH->>Repo: Run TypeScript typecheck
Repo-->>GH: Job results
GH-->>Dev: CI status (pass/fail)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/ci-web.yml (1)
15-60: Consider job optimization for future maintenance.While the current approach works, consider using a matrix strategy to reduce duplication if you add more jobs in future PRs.
Example matrix approach:
jobs: ci-checks: runs-on: ubuntu-latest strategy: matrix: check: [prettier, eslint, typecheck] steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: node-version: '24.3.0' cache: yarn cache-dependency-path: 'web/yarn.lock' - name: Install yarn run: npm install -g yarn - name: Install dependencies run: yarn install - name: Run check run: | case "${{ matrix.check }}" in prettier) yarn prettier . --check --ignore-unknown ;; eslint) yarn eslint . ;; typecheck) yarn tsc --noEmit --strict ;; esacweb/components/browse-matches-button.tsx (1)
44-45: Clarify the commenting approach.The partial commenting of the
introductionvariable assignment is unconventional. Consider either fully commenting out the lines or using a cleaner approach.Apply this diff for cleaner commenting:
- //const introduction = - ;(editor?.getCharacterCount() ?? 0) > 0 ? editor?.getJSON() : undefined + // const introduction = (editor?.getCharacterCount() ?? 0) > 0 ? editor?.getJSON() : undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/ci-web.yml(1 hunks)web/components/answers/compatibility-questions-display.tsx(2 hunks)web/components/answers/free-response-add-question.tsx(1 hunks)web/components/browse-matches-button.tsx(2 hunks)web/components/filters/location-filter.tsx(1 hunks)web/components/layout/tabs.tsx(1 hunks)web/components/love-page.tsx(2 hunks)web/components/required-lover-form.tsx(1 hunks)web/components/widgets/choices-toggle-group.tsx(1 hunks)web/components/widgets/searchable-select.tsx(1 hunks)web/components/widgets/user-link.tsx(1 hunks)web/pages/profile.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/pages/profile.tsx (2)
common/src/supabase/utils.ts (1)
Column(23-23)common/src/love/lover.ts (1)
LoverRow(5-5)
web/components/required-lover-form.tsx (2)
common/src/supabase/utils.ts (1)
Column(23-23)common/src/love/lover.ts (1)
LoverRow(5-5)
🔇 Additional comments (15)
.github/workflows/ci-web.yml (2)
3-9: LGTM! Well-configured workflow triggers.The path filters efficiently target only relevant changes in the web workspace while excluding documentation files from triggering CI checks.
11-13: LGTM! Efficient use of defaults.Setting the working directory globally eliminates repetition across jobs and ensures consistency.
web/components/widgets/user-link.tsx (1)
101-107: LGTM! Clean temporary disabling of unused component.The BotBadge function is properly commented out, preserving the implementation while removing it from active use. This aligns with the PR's goal of cleaning up unused code for CI checks.
web/components/widgets/searchable-select.tsx (1)
55-55: LGTM! Appropriate removal of unused variable.Removing the unused
openvariable from the destructuring helps clean up the code for ESLint checks while maintaining all functionality.web/components/layout/tabs.tsx (1)
160-163: LGTM! Improved JSX formatting consistency.The change from parentheses to curly braces for the multiline
clsx()expression follows better JSX conventions and improves code consistency.web/components/answers/compatibility-questions-display.tsx (1)
410-410: LGTM! CSS class ordering improvements.The reordering of CSS classes improves consistency and likely aligns with formatting tool preferences (Prettier) without affecting functionality.
Also applies to: 422-422
web/components/filters/location-filter.tsx (1)
31-31: LGTM - Aligns with PR objectives.Commenting out the unused
youLoverprop in the destructuring is appropriate for the eslint CI checks while preserving the component interface.web/components/answers/free-response-add-question.tsx (1)
25-25: LGTM - Consistent with CI cleanup objectives.Commenting out the unused
isFirstQuestionprop follows the established pattern for eslint compliance while maintaining the component interface.web/components/love-page.tsx (2)
48-48: LGTM - Unused modal state commented out.Commenting out the unused
isModalOpenstate is appropriate for eslint compliance.
58-58: LGTM - Clean destructuring pattern.Using the blank placeholder for the unused
isAddFundsModalOpenstate while keeping the setter is a clean approach for eslint compliance.web/components/widgets/choices-toggle-group.tsx (1)
16-18: LGTM - Improved formatting for complex generic types.The multiline formatting for the generic type constraint improves readability and aligns with the prettier formatting standards being introduced.
web/components/browse-matches-button.tsx (2)
41-41: LGTM - Unused variable commented out.Commenting out the unused
setIsSubmittingin destructuring is appropriate for eslint compliance.
110-114: Verify intentional disabling of submission functionality.Multiple props related to submission are commented out, which goes beyond simple eslint cleanup. Please confirm this is intentional disabling of match submission functionality.
web/components/required-lover-form.tsx (1)
50-53: LGTM! Formatting improvement for better readability.The multi-line function signature formatting enhances readability while maintaining identical type constraints and functionality. This change aligns well with the PR objective of adding prettier formatting checks to the CI pipeline.
web/pages/profile.tsx (1)
33-36: LGTM! Consistent formatting improvement.The multi-line function declaration formatting is consistent with the similar change in
web/components/required-lover-form.tsxand enhances code readability. The function logic and behavior remain unchanged.
.github/workflows/ci-web.yml
Outdated
| node-version: '24.3.0' | ||
| cache: npm | ||
| cache-dependency-path: 'package.json' |
There was a problem hiding this comment.
Fix cache configuration mismatch.
The jobs use npm caching but install and use yarn for package management. This cache configuration won't be effective.
Apply this diff to fix the cache configuration:
- cache: npm
- cache-dependency-path: 'package.json'
+ cache: yarn
+ cache-dependency-path: 'web/yarn.lock'Also applies to: 38-39, 53-54
🤖 Prompt for AI Agents
In .github/workflows/ci-web.yml at lines 22 to 24 (and similarly at lines 38-39
and 53-54), the cache configuration is set for npm while the project uses yarn
for package management. Update the cache configuration to use yarn caching by
changing 'cache: npm' to 'cache: yarn' and update 'cache-dependency-path' to
point to 'yarn.lock' instead of 'package.json' to ensure effective caching.
.github/workflows/ci-web.yml
Outdated
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '24.3.0' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Node.js version currency and command availability.
Ensure the specified Node.js version is current and that the expected commands are available in the web workspace.
Also applies to: 37-37, 52-52
🏁 Script executed:
#!/bin/bash
# Check if the specified Node.js version is current and verify package.json scripts
# Check if package.json exists in web directory
if [ -f "web/package.json" ]; then
echo "=== Checking package.json scripts ==="
jq '.scripts' web/package.json
echo -e "\n=== Checking if yarn.lock exists ==="
ls -la web/ | grep -E "(yarn\.lock|package-lock\.json)"
echo -e "\n=== Checking dependencies ==="
jq '.devDependencies | keys[]' web/package.json | grep -E "(prettier|eslint|typescript)"
else
echo "web/package.json not found"
fiLength of output: 1549
CI Web workflow: Pin LTS Node.js and install TypeScript
I verified the CI setup and package scripts in web/package.json:
- You’re currently pinning
node-version: '24.3.0'. That’s a valid current release but not an LTS line—if you need stability across environments, consider switching to the latest LTS (v20.x) or using a matrix to cover both LTS and current. yarn.lockis present and all your custom scripts (serve, ts-watch, dev, etc.) are defined as expected.- Missing TypeScript: I didn’t find
"typescript"underdevDependencies. Thetsccommand in yourts-watchscript will fail in CI unless you install it locally.
Please make the following updates:
- In
.github/workflows/ci-web.yml(lines 22, 37, 52), swap to an LTS version or use a matrix:node-version: ['20', '24.3.0']
- In
web/package.json, add TypeScript (and any related types) todevDependencies:"devDependencies": {
- "typescript": "^5.1.6",
"eslint-config-next": "...",
"eslint-config-prettier": "...",
"prettier-plugin-sql": "...",
"prettier-plugin-tailwindcss": "..."
}
<details>
<summary>🤖 Prompt for AI Agents</summary>
In .github/workflows/ci-web.yml at lines 22, 37, and 52, update the node-version
field to use a matrix with both the latest LTS version and the current version
by setting node-version to ['20', '24.3.0']. Additionally, in web/package.json,
add "typescript" and any necessary related type packages to the devDependencies
section to ensure the tsc command works correctly in CI.
</details>
<!-- fingerprinting:phantom:poseidon:tarantula -->
<!-- This is an auto-generated reply by CodeRabbit -->
There was a problem hiding this comment.
@coderabbitai your comment is formatted wrong. starting immediately after the place where it says "devDependencies": { the code block is rendered as plain markdown and vice versa. please fix this ASAP
There was a problem hiding this comment.
@duck-master You're absolutely right - my apologies for the formatting issue! Here's the corrected version:
CI Web workflow: Pin LTS Node.js and install TypeScript
I verified the CI setup and package scripts in web/package.json:
- You're currently pinning
node-version: '24.3.0'. That's a valid current release but not an LTS line—if you need stability across environments, consider switching to the latest LTS (v20.x) or using a matrix to cover both LTS and current. yarn.lockis present and all your custom scripts (serve, ts-watch, dev, etc.) are defined as expected.- Missing TypeScript: I didn't find
"typescript"underdevDependencies. Thetsccommand in yourts-watchscript will fail in CI unless you install it locally.
Please make the following updates:
In .github/workflows/ci-web.yml (lines 22, 37, 52), swap to an LTS version or use a matrix:
node-version: ['20', '24.3.0']In web/package.json, add TypeScript to devDependencies:
{
"devDependencies": {
"typescript": "^5.1.6",
"eslint-config-next": "...",
"eslint-config-prettier": "...",
"prettier-plugin-sql": "...",
"prettier-plugin-tailwindcss": "..."
}
}
|
@sipec do you know what version of node.js and yarn we are using in production? This might actually become important. (Adding typescript to devDependencies and using yarn caching should be easy.) |
|
interesting ... |
2 participants
This is my first PR in a series of (hopefully!) five PRs, each of which adds roughly the same set of CI checks (namely: prettier, eslint, tsc). I've also commented out the unused variables so that eslint will work and I have avoided touching the repo-level
yarn.lock.Summary by CodeRabbit
Chores
Refactor
Style