Add TrailingComment module to checkstyle #7138
Conversation
optionsome
added
!Technical Debt
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #7138 +/- ##
=============================================
- Coverage 72.17% 72.17% -0.01%
Complexity 20877 20877
=============================================
Files 2273 2273
Lines 84437 84439 +2
Branches 8424 8425 +1
=============================================
+ Hits 60942 60943 +1
Misses 20520 20520
- Partials 2975 2976 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
t2gran
left a comment
t2gran
left a comment
There was a problem hiding this comment.
There are a few comments witch can be removed. I added some suggestions for those, hopefully the code compiles ++ if applied.
Many comments can be replaced with constants replacing MAGIC numbers. But, I think that is beyond the scope of this PR.
I am ready to approve this PR, let me know when you have looked at my comments. If you choose to not apply the suggested changes that is ok. They are removing some of the comments witch I think does not provide any information.
| <module name="AvoidStarImport"/> | ||
| <module name="NeedBraces"/> | ||
| <module name="RedundantImport"/> | ||
| <module name="UnusedImports"/> | ||
| <module name="UnusedLocalVariable"/> | ||
| <module name="NeedBraces"/> | ||
| <module name="TrailingComment"> | ||
| <property name="format" value="^\s*$"/> | ||
| </module> |
There was a problem hiding this comment.
Consider: I would prefer to keep these in alphabetic order - less merge conflicts, easier to read if you are looking for something, avoid duplicates...
There was a problem hiding this comment.
I already moved that NeedBraces rules just for that purpose but forgot to do the same for the new rule... I've fixed it now.
...ation/src/ext-test/java/org/opentripplanner/ext/siri/updater/azure/SiriAzureUpdaterTest.java
Outdated
Show resolved
Hide resolved
...on/src/ext/java/org/opentripplanner/ext/fares/service/gtfs/v1/custom/AtlantaFareService.java
Outdated
Show resolved
Hide resolved
...cation/src/ext/java/org/opentripplanner/ext/fares/service/gtfs/v1/custom/HSLFareService.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/framework/geometry/DirectionUtils.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/framework/geometry/DirectionUtils.java
Outdated
Show resolved
Hide resolved
...ation/src/main/java/org/opentripplanner/graph_builder/module/islandpruning/PruneIslands.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/gtfs/mapping/TransitModeMapper.java
Outdated
Show resolved
Hide resolved
| // = 120 km/h. Varies between 80 - 120 km/h depending on road and season. | ||
| props.setCarSpeed("highway=motorway", 33.33f); | ||
| // = 54 km/h | ||
| props.setCarSpeed("highway=motorway_link", 15); | ||
| // = 100 km/h | ||
| props.setCarSpeed("highway=trunk", 27.27f); | ||
| // = 54 km/h | ||
| props.setCarSpeed("highway=trunk_link", 15); | ||
| // = 100 km/h | ||
| props.setCarSpeed("highway=primary", 27.27f); | ||
| // = 54 km/h | ||
| props.setCarSpeed("highway=primary_link", 15); |
There was a problem hiding this comment.
This should be replaced with constants (MAGIC NUMBERS) - witch will remove the need of explaining the value. I do not expect this to be done in this PR.
4 participants
Summary
Adds TrailingComment module to checkstyle and fixes the issues related to it.
Issue
Relates to #7068
Unit tests
No functional changes
Documentation
Not updated
Changelog
Skipped