feat: add GitHub Actions CI workflow for typecheck and lint#331
Merged
Sachinchaurasiya360 merged 6 commits intoMay 21, 2026
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe GitHub Actions CI workflow is streamlined to trigger only on pull requests to main. TypeScript typecheck jobs for client and server are restructured with explicit dependency installation and ChangesGitHub Actions CI/CD Workflow Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
9-91: ⚡ Quick winConsider using a matrix strategy to reduce duplication.
The four jobs share nearly identical structure. A matrix strategy could reduce duplication and improve maintainability:
♻️ Example matrix-based refactor
jobs: checks: name: ${{ matrix.check }} ${{ matrix.package }} runs-on: ubuntu-latest strategy: matrix: package: [client, server] check: [typecheck, lint] steps: - uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 20 cache: "npm" cache-dependency-path: ${{ matrix.package }}/package-lock.json - name: Install dependencies run: npm ci working-directory: ${{ matrix.package }} - name: Run typecheck if: matrix.check == 'typecheck' run: npx tsc --noEmit working-directory: ${{ matrix.package }} - name: Run lint if: matrix.check == 'lint' run: npx eslint . --ext ${{ matrix.package == 'client' && '.ts,.tsx' || '.ts' }} working-directory: ${{ matrix.package }}This approach reduces duplication while maintaining clarity. However, the current explicit job definitions are also valid and may be easier to customize per-job if needed in the future.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 9 - 91, The four nearly identical jobs (typecheck-client, typecheck-server, lint-client, lint-server) should be collapsed into a single matrix job to remove duplication: add a job (e.g., checks) with a strategy.matrix over package: [client, server] and check: [typecheck, lint], update shared steps (checkout, actions/setup-node with cache-dependency-path using ${{ matrix.package }}/package-lock.json, npm ci with working-directory: ${{ matrix.package }}), and replace the per-job run steps with conditional steps using if: matrix.check == 'typecheck' to run npx tsc --noEmit and if: matrix.check == 'lint' to run the appropriate eslint invocation (use .ts,.tsx for client when matrix.package == 'client' and .ts for server) so you preserve behavior of typecheck-client/typecheck-server and lint-client/lint-server while eliminating repeated blocks.client/src/components/Navbar.tsx (1)
49-56: 💤 Low valueConsider extracting role-based routing logic for readability.
The nested ternary is functionally correct but could be more readable. For a "Chill" review, this is purely optional.
♻️ Optional refactor using a helper function
+const getDashboardLink = (role: string | undefined) => { + if (role === "ADMIN") return "/admin"; + if (role === "RECRUITER") return "/recruiters"; + return "/student/applications"; +}; + +const getProfileLink = (role: string | undefined) => { + return role === "RECRUITER" ? "/recruiters/profile" : "/student/profile"; +}; + - const dashboardLink = - user?.role === "ADMIN" - ? "/admin" - : user?.role === "RECRUITER" - ? "/recruiters" - : "/student/applications"; - const profileLink = - user?.role === "RECRUITER" ? "/recruiters/profile" : "/student/profile"; + const dashboardLink = getDashboardLink(user?.role); + const profileLink = getProfileLink(user?.role);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/Navbar.tsx` around lines 49 - 56, Extract the role-based routing logic out of the nested ternary expressions into a small helper (e.g., getDashboardLink(role) and getProfileLink(role) or a single getRouteForRole(role)) and use that from the Navbar component instead of the inline ternaries that set dashboardLink and profileLink; implement the helper using a switch or an object map to return "/admin", "/recruiters", or "/student/applications" for dashboardLink and "/recruiters/profile" or "/student/profile" for profileLink, and call it with user?.role in Navbar to improve readability and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/module/student/jobs/JobBrowsePage.tsx`:
- Around line 140-144: When debounced filters change (debouncedSearch,
debouncedLocation) or the submit handler is invoked (the submit/handleSubmit
function around lines 327–330), reset pagination state to avoid landing on
empty/misaligned pages: set page, extPage, and scrPage back to 1 before
triggering the refetch. Update the effect(s) that depend on debouncedSearch and
debouncedLocation to call setPage(1), setExtPage(1), and setScrPage(1) prior to
any fetch, and ensure the submit handler also resets those three states before
performing its fetch/refetch logic.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 9-91: The four nearly identical jobs (typecheck-client,
typecheck-server, lint-client, lint-server) should be collapsed into a single
matrix job to remove duplication: add a job (e.g., checks) with a
strategy.matrix over package: [client, server] and check: [typecheck, lint],
update shared steps (checkout, actions/setup-node with cache-dependency-path
using ${{ matrix.package }}/package-lock.json, npm ci with working-directory:
${{ matrix.package }}), and replace the per-job run steps with conditional steps
using if: matrix.check == 'typecheck' to run npx tsc --noEmit and if:
matrix.check == 'lint' to run the appropriate eslint invocation (use .ts,.tsx
for client when matrix.package == 'client' and .ts for server) so you preserve
behavior of typecheck-client/typecheck-server and lint-client/lint-server while
eliminating repeated blocks.
In `@client/src/components/Navbar.tsx`:
- Around line 49-56: Extract the role-based routing logic out of the nested
ternary expressions into a small helper (e.g., getDashboardLink(role) and
getProfileLink(role) or a single getRouteForRole(role)) and use that from the
Navbar component instead of the inline ternaries that set dashboardLink and
profileLink; implement the helper using a switch or an object map to return
"/admin", "/recruiters", or "/student/applications" for dashboardLink and
"/recruiters/profile" or "/student/profile" for profileLink, and call it with
user?.role in Navbar to improve readability and maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24c185d2-93f7-44a6-9ce3-88ef52fe714a
📒 Files selected for processing (4)
.github/workflows/ci.ymlclient/src/components/Navbar.tsxclient/src/module/student/companies/CompanyListPage.tsxclient/src/module/student/jobs/JobBrowsePage.tsx
Sachinchaurasiya360
added
bug
Sachinchaurasiya360
merged commit afec3b1
into
Sachinchaurasiya360:main
0 of 4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #103
What
Adds a new GitHub Actions CI workflow at
.github/workflows/ci.ymlthat runs on every pull request targeting
main.Jobs
typecheck-client→ runstsc --noEmitin/clienttypecheck-server→ runstsc --noEmitin/serverlint-client→ runs ESLint on.ts/.tsxfiles in/clientlint-server→ runs ESLint on.tsfiles in/serverFeatures
node_modulescached viaactions/setup-nodefor faster runsFiles Changed
.github/workflows/ci.yml(new file)Label Request
Requesting
enhancement+devopslabels.Summary by CodeRabbit