fix: honor custom dimensions in default streak badge#5328
Conversation
|
@Tanayajadhav1 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 @Tanayajadhav1, 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 contribution. I went through the changes and the overall approach to fixing the custom dimensions looks correct, but there is a major issue that needs to be addressed before this can be merged.
It appears that a significant portion of unrelated code, specifically the entire \generateSkylineSVG\ function in \lib/svg/generator.ts, was accidentally deleted in this PR. Please revert the deletion of \generateSkylineSVG\ and ensure only the code related to the default streak badge dimensions is modified.
Labels Applied:
- \level:intermediate: The intended bug fix touches the core SVG generation logic and requires understanding the parameter handling.
- \ ype:bug: This PR aims to fix a bug where custom dimensions were ignored.
- \mentor:Aamod007\
Once the unrelated deletions are reverted and the fix is isolated, I'll be happy to take another look. Thanks again for the contribution.
|
Review Update |
Aamod-Dev
left a comment
There was a problem hiding this comment.
This PR is currently marked with the \status:blocked\ label. Please resolve the blockers so we can proceed with a full review and approval.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I went through the changes and have evaluated them according to the rubric.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Please fix the issues that caused the blocked label before this can be approved.
Aamod-Dev
left a comment
There was a problem hiding this comment.
This PR is currently blocked due to a failing CI check or other blocking issues. Please fix the blocking issues so we can proceed with the review and approval process.
Aamod-Dev
left a comment
There was a problem hiding this comment.
This is blocked, so I can’t approve it yet. The dimension handling looks like the right direction, but lib/svg/generator.ts is doing a very broad rewrite and the PR is still marked blocked. Please clear the blocker, then add a focused test proving the custom width and height values are respected without breaking the existing default badge paths.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Difficulty: beginner – Bug fix implementation.
Quality: clean – Solid fix.
Type: bug – Issue resolution.
Good fix!
Description
Fixes #5321
This PR fixes an inconsistency in the default streak badge dimension handling.
Currently, the API accepts and validates custom
widthandheightparameters for default streak badge requests, but the generated SVG ignores those values and continues to render using the hardcoded600x420dimensions.Changes made
widthandheightvalues.Example
Before:
/api/streak?user=octocat&width=777&height=666
Rendered:
viewBox="0 0 600 420"
After:
viewBox="0 0 777 666"
Pillar
Visual Preview
Before
viewBox="0 0 600 420"
After
viewBox="0 0 777 666"
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).README.mdif I added a new theme or URL parameter. (Not applicable)suggested labels :
gssoc:approved , gssoc'26 , bug , level:intermediate