-
Notifications
You must be signed in to change notification settings - Fork 63
Sonarqube cleanup #2738
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
base: main
Are you sure you want to change the base?
Sonarqube cleanup #2738
Conversation
The '[[' construct is safer and more feature-rich, preventing word splitting and pathname expansion issues. Scripts that run in Docker containers with /bin/sh (Alpine/busybox) remain unchanged to maintain POSIX compliance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- logger.py: Rename DEBUG/INFO/WARN/ERROR/FATAL to DEBUG_VALUE/INFO_VALUE/ WARN_VALUE/ERROR_VALUE/FATAL_VALUE to avoid clash with method names. Keep old names as backwards compatibility aliases. - packet_item.py: Rename STATE_COLORS to VALID_STATE_COLORS to avoid clash with state_colors instance attribute. - packet_config.py: Rename COMMAND/TELEMETRY to COMMAND_STRING/ TELEMETRY_STRING to avoid clash with commands/telemetry attributes. - Openc3Screen.vue: Add "falls through" comments for intentional switch case fallthrough in SUBSETTING and GLOBAL_SUBSETTING cases. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Thread-local storage is used intentionally for: - Pipeline context tracking in Redis operations - Topic offset tracking per-thread for stream reading These are well-established Ruby patterns with proper cleanup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2738 +/- ##
==========================================
- Coverage 79.22% 78.30% -0.93%
==========================================
Files 670 473 -197
Lines 54178 34451 -19727
Branches 734 734
==========================================
- Hits 42924 26977 -15947
+ Misses 11174 7394 -3780
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Can we still use INFO and WARN to set the log level or do we need to use INFO_VALUE and WARN_VALUE?
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.
Sonar didn't like the constant INFO shadowing the method info. So I created aliases for INFO but yeah you would now use INFO_VALUE ... I don't really like that though. The INFO_LEVEL constant was already declared to be the string "INFO" ... that actually makes more sense as setting level = Logger.INFO_LEVEL. I don't like level = Logger.INFO_VALUE.
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.
Could this be clearer and avoid the shadowing problem by implementing the log levels as an enum, rather than as 5 separate variables?
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.
Would something like this work?
Read the commit messages to know what I did and why