-
Notifications
You must be signed in to change notification settings - Fork 0
fix: change textureId to texture_id to follow zig convention, replace 9999… #216
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
… with Number.MAX_SAFE_INTEGER
WalkthroughRenames serialized image field textureId to texture_id across public types and internal serialization, updates tests to match, and changes a gradient fill sentinel value from Infinity to Number.MAX_SAFE_INTEGER in SVG shape data processing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
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.
Pull Request Overview
This pull request standardizes naming conventions and improves numeric constant usage by converting camelCase to snake_case for consistency with Zig conventions and replacing magic numbers with more semantically appropriate constants.
- Changed
textureIdtotexture_idacross type definitions and implementations - Replaced hardcoded
InfinitywithNumber.MAX_SAFE_INTEGERfor better numeric precision
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/index.ts | Updated type definitions and property references from textureId to texture_id |
| src/svgToShapes/collectShapesData.ts | Replaced Infinity with Number.MAX_SAFE_INTEGER for distance calculations |
| integration-tests/tests/assets-snapshot.spec.ts | Updated test data to use texture_id instead of textureId |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
integration-tests/tests/assets-snapshot.spec.ts(1 hunks)src/index.ts(4 hunks)src/svgToShapes/collectShapesData.ts(1 hunks)
🔇 Additional comments (5)
src/index.ts (4)
15-20: LGTM! Breaking API change for snake_case convention.The rename from
textureIdtotexture_idaligns with Zig naming conventions. Note that this is a breaking change for any consumers of theSerializedInputImagetype who may be passing or reading thetextureIdproperty.
41-46: LGTM! Consistent with input type rename.The rename maintains consistency across both
SerializedInputImageandSerializedOutputImagetypes.
184-192: LGTM! Serialization correctly uses renamed field.The internal serialization properly maps to the renamed
texture_idfield, maintaining consistency with the updated types.
296-303: Ignore internal textureId occurrences
All remainingtextureIdmatches are local variables or function parameters, not serialized fields. The API payload correctly usestexture_id. No further renaming required.Likely an incorrect or invalid review comment.
integration-tests/tests/assets-snapshot.spec.ts (1)
7-39: LGTM! Test snapshot correctly reflects API changes.The test snapshot has been properly updated to expect
texture_idinstead oftextureId, ensuring tests align with the renamed public API.
|
🎉 This PR is included in version 0.1.0-next.23 🎉 The release is available on: Your semantic-release bot 📦🚀 |
… with Number.MAX_SAFE_INTEGER
Summary by CodeRabbit