Fix wrong rounding with double/float values on x/y#130
Conversation
WalkthroughThis update introduces two new Git hook scripts—one for pre-commit and one for pre-push—to automate code quality checks and tests. The pre-commit hook runs Detekt static analysis for both the library and plugin portions of the project, while the pre-push hook executes a suite of tests. Additionally, the GitHub workflow for pull requests has been adjusted to run Detekt on two project components, the application version has been incremented in the properties file, and several functions and tests in the SVG-to-compose module have been updated to use floating-point coordinates instead of integers. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Git as Git
participant PreCommit as pre-commit.sh
participant Detekt as Detekt Tool
Dev->>Git: Initiate commit
Git->>PreCommit: Trigger pre-commit hook
PreCommit->>Detekt: Run Detekt for library analysis
Detekt-->>PreCommit: Library analysis result
PreCommit->>Detekt: Run Detekt for plugin analysis
Detekt-->>PreCommit: Plugin analysis result
PreCommit-->>Git: Return status (pass/fail)
sequenceDiagram
participant Dev as Developer
participant Git as Git
participant PrePush as pre-push.sh
participant Gradle as Gradle Test Runner
Dev->>Git: Initiate push
Git->>PrePush: Trigger pre-push hook
PrePush->>Gradle: Run tests (“./gradlew cleanAllTests allTests”)
Gradle-->>PrePush: Test results
PrePush-->>Git: Allow push or abort based on results
sequenceDiagram
participant PR as Pull Request
participant Workflow as GitHub Workflow
participant Gradle as Gradle Runner
PR->>Workflow: Open pull request
Workflow->>Gradle: Run "detektMetadataCommonMain" (svg-to-compose)
Gradle-->>Workflow: Return analysis output
Workflow->>Gradle: Run "detekt" (svg-to-compose-gradle-plugin)
Gradle-->>Workflow: Return analysis output
Workflow-->>PR: Report analysis results
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.githooks/pre-commit.sh (2)
3-14: Output File Creation Function
Thefn_create_output_filefunction correctly converts the input type to lowercase and constructs unique log file paths based on the current timestamp.
- Suggestion: Before attempting to write to the log file, ensure the target directory exists to avoid any file creation issues.
- BASE_DIR="./build/reports/detekt" + BASE_DIR="./build/reports/detekt" + mkdir -p "$BASE_DIR"
16-37: Detekt Check Execution Function
Thefn_run_detektfunction dynamically sets the Gradle task based on the type (usingdetektMetadataCommonMainfor "library" and defaulting todetektfor "plugin"), captures the output in a log file, and handles failures appropriately.
- Suggestion: To capture all error details from Gradle, consider redirecting both stdout and stderr to the log file.
- ./gradlew "$GRADLE_TASK" > "$OUTPUT_FILE" + ./gradlew "$GRADLE_TASK" > "$OUTPUT_FILE" 2>&1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.githooks/pre-commit.sh(1 hunks).githooks/pre-push.sh(1 hunks).github/workflows/pull_request.yml(1 hunks)app.properties(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/StrokeDashArray.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRectNode.kt(5 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgRectNodeTests.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app.properties
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-check (ubuntu-latest)
- GitHub Check: build-and-check (windows-latest)
- GitHub Check: build-and-check (macos-latest)
🔇 Additional comments (25)
.githooks/pre-push.sh (3)
1-2: Shebang and Function Declaration
The shebang (#!/bin/sh) and the start of thefn_mainfunction definition are set up correctly for portability in a shell environment.
3-12: Pre-push Hook Test Execution Logic
The hook properly runs tests via./gradlew cleanAllTests allTestsand checks for a non-zero exit code to abort the push when tests fail. Consider, as an optional improvement, redirecting stderr to the output (e.g., using2>&1) if you require more detailed diagnostic logs in the terminal.
14-15: Invocation of the Main Function
Callingfn_mainat the end ensures the hook executes as expected..github/workflows/pull_request.yml (2)
56-60: Renaming and Running Detekt on svg-to-compose
The renamed step ("Running Detekt on svg-to-compose") now clearly reflects the command./gradlew detektMetadataCommonMainintended for static analysis on the main project component.
61-65: Adding Detekt Check for the Gradle Plugin
Introducing a new step to run./gradlew detektfor the svg-to-compose-gradle-plugin is a valuable addition. This ensures that both project components are statically analyzed..githooks/pre-commit.sh (2)
1-2: Initialization and Constant Declaration
The file starts with a correct shebang and the declaration ofINVALID_OUTPUT_FILE_ERROR_CODE. This enhances clarity and sets up the basis for consistent error handling.
40-48: Main Function Orchestration
Thefn_mainfunction clearly orchestrates the execution of Detekt checks for both "library" and "plugin" types. Its sequential structure ensures that each check is performed before committing the changes.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgRectNodeTests.kt (2)
19-20: Correctly updated test assertions to use floating-point valuesThe test assertions have been appropriately updated to expect floating-point values (10f and 20f) instead of integers, aligning with the type changes in the SvgRectNode class.
19-20: Good update to the test expectations for float values.The test expectations have been correctly updated to use float literals (10f, 20f) instead of integers, consistent with the implementation changes in SvgRectNode.kt where x and y were changed from Int? to Float?.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/StrokeDashArray.kt (2)
30-31: Function signature updated to use float parametersThe parameter types of
xandyhave been appropriately changed fromInttoFloatto support more precise positioning and rounding. This change aligns with similar modifications in SvgRectNode.
29-32: Parameter type change improves precision.Changing the x and y parameters from Int to Float allows for more precise positioning of dashed paths, which addresses the issue with wrong rounding for double/float values mentioned in the PR objectives.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRectNode.kt (14)
28-32: Property type updated from Int to Float for x coordinateThe
xproperty has been correctly changed fromInt?toFloat?and the conversion function fromtoIntOrNulltotoFloatOrNull. This change enhances coordinate precision for SVG elements.
33-37: Property type updated from Int to Float for y coordinateThe
yproperty has been correctly changed fromInt?toFloat?and the conversion function fromtoIntOrNulltotoFloatOrNull. This change enhances coordinate precision for SVG elements.
71-72: Default values updated to match new Float typeThe default values for
xandyhave been appropriately updated from0to0fto match the newFloattype.
90-95: Function parameter types updated to Float for createDashedRectThe parameter types for
xandyin thecreateDashedRectfunction have been correctly updated fromInttoFloat, maintaining consistency with the property type changes.
110-114: Function parameter types updated to Float for createRegularRectThe parameter types for
xandyin thecreateRegularRectfunction have been correctly updated fromInttoFloat, maintaining consistency with the property type changes.
137-143: Function parameter types updated to Float for createRoundedCornerRectThe parameter types for
xandyin thecreateRoundedCornerRectfunction have been correctly updated fromInttoFloat, maintaining consistency with the property type changes.
28-193:Details
✅ Verification successful
Verify all call sites using x/y coordinates
The change from
InttoFloatfor x/y coordinates is well-implemented in this file, but we should verify that all call sites using these coordinates have been updated accordingly.
🏁 Script executed:
#!/bin/bash # Description: Find all places in the codebase where SvgRectNode's x and y properties are accessed or where the modified functions are called # Search for direct references to SvgRectNode.x and SvgRectNode.y echo "### Searching for references to SvgRectNode.x and SvgRectNode.y properties:" rg -A 3 "SvgRectNode.*\.(x|y)" --type kotlin # Search for calls to the updated functions echo -e "\n### Searching for calls to updated functions:" rg -A 3 "(createDashedRect|createRegularRect|createRoundedCornerRect).*x,.*y" --type kotlin # Look for potential type mismatches (Int vs Float) when working with coordinates echo -e "\n### Searching for potential type mismatches with coordinates:" rg -A 3 "(x|y)(\s*:\s*Int|\s*=\s*\d+[^f])" --type kotlin | grep -v "width" | grep -v "height"Length of output: 16729
Action Verified: x/y Coordinate Call Sites are Consistent
The updated implementation in
SvgRectNode.ktusingFloat?for bothxandyhas been verified against all related call sites. Our search confirms that:
- All calls to
createDashedRect,createRegularRect, andcreateRoundedCornerRectcorrectly passxandyasFloatvalues.- Test cases (e.g., in
SvgPathLineToTests.kt,SvgPathHorizontalLineToTests.kt, etc.) consistently use floating point values (e.g.,80f).- No type mismatches were detected in the repository for the
xandycoordinates inSvgRectNode.These findings indicate that the refactoring from
InttoFloatforxandyis applied uniformly and without adverse effects.
28-32: Type change from Int to Float fixes rounding issues.Changing x from Int? to Float? and updating the parsing method from toIntOrNull to toFloatOrNull preserves fractional position values, addressing the rounding issue mentioned in the PR objectives.
33-37: Type change from Int to Float fixes rounding issues.Changing y from Int? to Float? and updating the parsing method from toIntOrNull to toFloatOrNull preserves fractional position values, addressing the rounding issue mentioned in the PR objectives.
71-72: Default values updated to match new type.The default values for x and y have been correctly updated to use float literals (0f) to match the new Float type, maintaining type consistency.
90-95: Parameter type change maintains consistency across codebase.The parameter types for x and y in createDashedRect have been updated to Float to align with the property type changes, ensuring type consistency.
110-113: Parameter type change maintains consistency across codebase.The parameter types for x and y in createRegularRect have been updated to Float to align with the property type changes, ensuring type consistency.
137-142: Parameter type change maintains consistency across codebase.The parameter types for x and y in createRoundedCornerRect have been updated to Float to align with the property type changes, ensuring type consistency.
200-201:Details
✅ Verification successful
Verify remaining non-primitive operations with Float values.
The string interpolation for x and y values should work fine with Float values, but it's worth verifying that any other operations (formatting, transformations, etc.) throughout the codebase that might interact with these values handle Float values correctly.
🏁 Script executed:
#!/bin/bash # Search for any code that uses x and y properties from SvgRectNode to ensure compatibility with Float values echo "Searching for usages of SvgRectNode.x and SvgRectNode.y..." rg -A 2 -B 2 "\.x\b|\.y\b" --type kotlin | grep -v "Float" | grep -v "SvgRectNode\.kt" || echo "No potential type incompatibilities found."Length of output: 54527
Float Operations Verified for SvgRectNode Properties
After reviewing the test outputs and scanning through the codebase, all non-primitive operations that interact with the Float values for
SvgRectNode.xandSvgRectNode.yare correctly handling these values:
- String Interpolation: The code snippet using string interpolation for the x and y values is working as expected.
- Conversions & Transformations: Various usages across the codebase (e.g., explicit conversions via
toDouble()in arithmetic and transformation operations) correctly work with Float values.- Testing Confirmation: The tests in
SvgRectNodeTests.ktconfirm that both non-null and null cases for these properties behave as intended without type incompatibilities.No further action is required regarding Float handling in these operations.
1 participant
Summary by CodeRabbit
New Features
Chores
Refactor
Tests