Conversation
Once I've removed sonar-batch artifact I do not longer need `core.properties` to get locales work when use this plugin in SQ 6.2
- Updated dependencies to work with SonarQube 6.7 - Removed Colorizer because it relies upon libraries that have been removed.
- Update Qualifiers to use current idenfiers - Remove use of deprecated Project class - Use TestInputFileBuilder to build files for testing - Remove commented code
- Update Qualifiers to use the updated identifier.
- Remove unused code - Correctly implement 6.7 interface - Update tests to use new libraries - Update tests to test the updated interface implementation
- Use new libaries and interfaces for 6.7 - Update test with new libraries - Needed to remove the test for execute() because EasyMock does not work with the return type by default - Information on how to fix the test here: - https://stackoverflow.com/q/2667172 - https://stackoverflow.com/q/7204829
- Use new libaries and interfaces for 6.7 - Update test with new libraries - Needed to change the test for execute() because the underlying library for saving metrics has changed.
- Update test with new libraries
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.
There are some minor changes that relate to documentation and comments and I didn't perform such review for the simplecov sensor out of principal(the old code-base was a barren wild-lands of code practices) so after those minor changes I think it's good. Also, I've fixed the Travis issue and added some minor changes to the README including your name 👍 🥇
} | ||
|
||
protected void computeBaseMetrics(SensorContext sensorContext, Project project) { | ||
protected void computeBaseMetrics(SensorContext sensorContext) { |
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.
add function docstring for non-API function. See[1]
[1] https://github.com/shakedlokits/ruby-sonar-plugin/blob/6.7-dev/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuComplexitySensor.java#L83
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.
Added.
@@ -1,7 +1,7 @@ | |||
package com.godaddy.sonar.ruby; | |||
|
|||
import com.godaddy.sonar.ruby.core.Ruby; | |||
import com.godaddy.sonar.ruby.core.RubySourceCodeColorizer; | |||
// import com.godaddy.sonar.ruby.core.RubySourceCodeColorizer; |
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.
If you removed the RubySourceCodeColorizer file, the comment reference should be removed as well
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.
Yup it should be.....and it's removed.
.setType(type) | ||
.build(); | ||
|
||
System.out.println(moduleBaseDir.toString() + relativePath); |
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.
Remove system logging and move debug logging to the logger if relevant. See[1]
[1] https://github.com/shakedlokits/ruby-sonar-plugin/blob/6.7-dev/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuComplexitySensor.java#L70
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.
Was only relevant while updating the test, I forgot to remove it with the other debugging statements. Removed.
By semantic versioning 2.0, minor and patch digits should be reset after major update
- Remove commented code - Add docstring - Remove development debug statement
No description provided.