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
Feature/1066/attribute descriptor for sourcecodeparser #3166
Feature/1066/attribute descriptor for sourcecodeparser #3166
Conversation
@@ -10,7 +10,7 @@ fun getAttributeDescriptors(): Map<String, AttributeDescriptor> { | |||
descriptors["classes"] = AttributeDescriptor(title = "Number of Classes", description = "Number of classes", link = ghLink) | |||
descriptors["functions_per_class"] = AttributeDescriptor(title = "Functions per Class", description = "Number of functions per class", link = ghLink) | |||
descriptors["average_statements_per_function"] = AttributeDescriptor(title = "Average Statements per Function", description = "Average number of statements per method", link = ghLink) | |||
descriptors["max_function_mcc"] = AttributeDescriptor(title = "Maximum Cyclic Complexity", description = "Maximum cyclic complexity of a function", link = ghLink) | |||
descriptors["max_function_mcc"] = AttributeDescriptor(title = "Maximum Cyclic Complexity", description = "Maximum cyclic complexity based on paths through a function by McCabe", link = ghLink) |
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.
In metricTitles.ts (old: metricNames.ts) the title for mcc ist "Cyclomatic Complexity". Either change it here or there to avoid inconsistency.
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.
Fair point, I will rename the title in this case to "Function Complexity" to bring it on par with SonarImporter.
Generally, it is important to remember, that the definitions of two importers for a metric can differ, even if they are seemingly identical. In this case I tried to find a middlepoint between mcc and function_cc, because SourceMonitor does not provide another term for 'general' complexity. And it is difficult to find out which term is exactly correct for this importer, because the documentation is rather lackluster.
Also I would 'abandon' metricNames.ts, because it is just the fallback for a handful of metrics which are not described yet.
If there is a major difference in the definitions between those two, I might change the .ts fallback.
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.
@phanlezz please remember that we can also have a completely independent file that has been generated by something else than our own importers/parsers. Thus, we probably have to keep the fallback mechanism in place as we can not determine what they define.
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.
@BridgeAR Yes, the fallback should stay ofcourse. My point is, that we don't need to update the definitions in the .ts or add more, since we added the attributeDescriptors to have a more flexible solution.
descriptors["functions"] = AttributeDescriptor(title = "Number of Functions", description = "Number of functions", link = npmLink) | ||
descriptors["classes"] = AttributeDescriptor(title = "Number of Classes", description = "Number of classes", link = npmLink) | ||
descriptors["lines_of_code"] = AttributeDescriptor(title = "Lines of Code", description = "Lines of code including empty lines and comments", link = npmLink) | ||
descriptors["comment_lines"] = AttributeDescriptor(title = "Comment Lines", description = "Number of lines containing either comment or commented-out code", link = npmLink) | ||
descriptors["comment_lines"] = AttributeDescriptor(title = "Comment Lines", description = "Number of lines containing either a comment or commented-out code", link = npmLink) |
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.
In metricTitle.js: "Number of Code Lines with Comments"
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.
I prefer the current title, because it is shorter and still understandable.
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.
LGTM - just take a look at metricTitles.ts and maybe change those two lines
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed! |
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
Attribute Descriptors for SourceCodeParser
Issue: #1066
Description
This adds attribute descriptors to the SourceCodeParser, and refactors some wordings in other descriptors.