CI Cleanup #54
CI Cleanup #54
Conversation
…adle build reports as artifacts on failed builds
WalkthroughThe GitHub Actions workflow was updated: branch filter syntax simplified to compact arrays; checkout upgraded v4→v5 and setup-java v3→v5; Java version/distribution quoting standardized; Gradle permission step replaced with a Gradle setup action; build now runs via the Gradle wrapper ( 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/gradle.ymlgradlew
🔇 Additional comments (5)
.github/workflows/gradle.yml (5)
5-5: LGTM: Cleaner branch filter syntax.The compact array format is more concise and equally valid.
Also applies to: 7-7
24-25: LGTM: Standardized quoting.Consistent string quoting improves readability with no functional change.
27-28: LGTM: Modern Gradle setup.The
gradle/actions/setup-gradle@v5action handles Gradle wrapper permissions automatically, addressing the PR objective to "make gradlew executable" without requiring an explicitchmodstep.
31-31: LGTM: Direct Gradle invocation is clearer.Using
./gradlew builddirectly is more straightforward than wrapping it in another action.
17-17: Updateupload-artifactfrom v4 to v5 or v6 for consistency with other actions.All action versions are valid. However,
upload-artifact@v4is inconsistent—v5.0.0 and v6.0.0 are available, while all other actions use v5. Align this action to match, or document why v4 is pinned (e.g., compatibility constraints).
| - name: Upload Build Artifacts | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| arguments: build | ||
| name: reports | ||
| path: build/reports/ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more descriptive artifact name.
The failure-triggered artifact upload works correctly, but the name "reports" is generic. Consider something more specific like "build-reports" or "gradle-reports" for easier identification when multiple artifacts exist.
♻️ Suggested improvement
- name: Upload Build Artifacts
if: failure()
uses: actions/upload-artifact@v4
with:
- name: reports
+ name: build-reports
path: build/reports/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Upload Build Artifacts | |
| if: failure() | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| arguments: build | |
| name: reports | |
| path: build/reports/ | |
| - name: Upload Build Artifacts | |
| if: failure() | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: build-reports | |
| path: build/reports/ |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/constants/CameraConstants.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/constants/CameraConstants.java
🧠 Learnings (2)
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/constants/CameraConstants.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/constants/CameraConstants.java
🪛 GitHub Actions: Java CI with Gradle
src/main/java/frc/trigon/robot/constants/CameraConstants.java
[error] 18-18: incompatible types: int cannot be converted to Transform3d. (Gradle compileJava error during ./gradlew build)
| public static final ObjectDetectionCamera OBJECT_DETECTION_CAMERA = new ObjectDetectionCamera( | ||
| "ObjectDetectionCamera", | ||
| ROBOT_CENTER_TO_OBJECT_DETECTION_CAMERA | ||
| 0 |
There was a problem hiding this comment.
Type mismatch causes compilation failure.
The constructor expects a Transform3d but receives int. Based on the commit message ("add error to check build artifact"), this appears intentional to test CI artifact upload on failure. If so, revert this change before merging—use ROBOT_CENTER_TO_OBJECT_DETECTION_CAMERA instead of 0.
Fix
- 0
+ ROBOT_CENTER_TO_OBJECT_DETECTION_CAMERA📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 0 | |
| ROBOT_CENTER_TO_OBJECT_DETECTION_CAMERA |
🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 18-18: incompatible types: int cannot be converted to Transform3d. (Gradle compileJava error during ./gradlew build)
make
gradlewexecutable, github actions tools version bumps, add gradle build reports as artifacts on failed builds