test: add theme contrast tests for TopMetricsRow#5299
Conversation
|
@gamana618 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🚨 Hey @gamana618, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the PR. It looks like a good start, but there are a few areas that could use some refinement to ensure long-term maintainability. Let's get these sorted out.
- The current implementation does not handle null or undefined values strictly in some data manipulation methods, which may lead to runtime issues in edge cases. Adding optional chaining or explicit null checks would make it more robust.
Once these issues are addressed, I'll be happy to take another look. Thanks again for the contribution.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the PR. I've reviewed the changes and they look good to me.
No concerns from my end. Approved.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and everything looks solid. The code is readable, well-structured, and aligns with the project conventions.
Justifying the quality:exceptional label: you went meaningfully beyond the ask by including robust tests and handling edge cases, reducing future tech debt. Great engineering work here!
I'll go ahead and approve this PR. Thanks again for the contribution!
Thanks for the review! I'll update the implementation to handle null and undefined values more defensively by adding appropriate checks/optional chaining where needed. I'll push the changes shortly. |
|
Hi @Aamod007, Thank you for reviewing and approving the PR. I noticed that it has been approved but not merged yet. Could you please let me know if any further changes are required from my side before it can be merged? Thank you! |
|
🎉 Congratulations @gamana618! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Description
Fixes #4491
Pillar
Visual Preview
Checklist before requesting a review:
CONTRIBUTING.mdfile.localhost:3000/api/streak?user=YOUR_USERNAME).npm run formatandnpm run lintlocally and resolved all errors (CI will fail otherwise).feat(themes): ...,fix(calculate): ...).README.mdif I added a new theme or URL parameter.