-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add more CI #18
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
|
Warning Rate limit exceeded@AndrewHanasiro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis PR updates CI to run a separate lint job and uses Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/continuous_integration.yml(1 hunks)README.md(2 hunks)package.json(2 hunks)sonar-project.properties(1 hunks)src/ambient.d.ts(1 hunks)
🔇 Additional comments (6)
package.json (2)
9-9: Verify that type-checking alone is sufficient for the Build job.The
build:checkscript validates TypeScript and Svelte code without producing build artifacts. While this is valuable for CI, the Build job in the workflow (line 69 of continuous_integration.yml) now only type-checks rather than actually building the application.Consider whether the Build job should run
npm run build(which produces artifacts) in addition to or instead ofbuild:check, or rename the CI job to "Type Check" for clarity.
32-32: Good addition for TypeScript support.Adding
@types/qrcodeprovides type definitions for theqrcodelibrary already in dependencies. The versions are compatible: qrcode latest version is 1.5.4, and @types/qrcode latest version is 1.5.6, with both falling within the same minor version range (1.5.x).Note: TypeScript users using @types/qrcode will need to add a // @ts-ignore above the data segment because it expects data: string. Be aware of this requirement when using the qrcode library with TypeScript.
.github/workflows/continuous_integration.yml (1)
71-88: LGTM! Good separation of concerns.The new Lint job follows the established pattern and properly separates linting from testing and building. The job structure is consistent with the other jobs in the workflow.
src/ambient.d.ts (1)
1-5: LGTM! Standard ambient declaration for Svelte components.This ambient module declaration correctly enables TypeScript to recognize
.sveltefile imports. The syntax follows the standard pattern for TypeScript + Svelte projects and supports thebuild:checkscript added to the CI workflow.README.md (2)
1-3: LGTM! Clear project identification.The title and description updates appropriately reflect the Auth+ Client project identity.
7-7: LGTM! Good addition for coverage visibility.The SonarCloud coverage badge provides quick visibility into test coverage metrics.
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: 4
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sonar-project.properties(1 hunks)src/routes/Menu.svelte(1 hunks)test/Menu.svelte.test.ts(1 hunks)vite.config.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Continuous Integration on Pull Request
src/routes/Menu.svelte
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
test/Menu.svelte.test.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🔇 Additional comments (3)
src/routes/Menu.svelte (1)
26-26: Good addition of test identifier.Adding the
data-testidattribute is a best practice for component testing and properly supports the test assertions intest/Menu.svelte.test.ts.Note: Please address the Prettier formatting warning flagged by the pipeline.
sonar-project.properties (1)
12-12: LCOV coverage path correctly aligned with Vitest configuration.The LCOV coverage path
./coverage/lcov.infocorrectly matches the Vitest configuration invite.config.ts, which specifiesreporter: ['html', 'lcov']andreportsDirectory: './coverage'. The previous concern about Clover format has been resolved by using LCOV throughout.test/Menu.svelte.test.ts (1)
9-22: Test assertions are well-structured.The test properly verifies navigation links, their href attributes, and the logout button's presence and initial disabled state.
Note: Please address the Prettier formatting warning flagged by the pipeline.
Summary by CodeRabbit
Documentation
Chores
Tests
Style/Types
✏️ Tip: You can customize this high-level summary in your review settings.