Skip to content
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

Add untracked metrics to suspiciousMetricsHelper.ts #2962 #2996

Merged
merged 12 commits into from Aug 31, 2022

Conversation

jannikr
Copy link
Contributor

@jannikr jannikr commented Aug 25, 2022

Suspicious Metrics Overlay is empty for maps without known metrics

Issue: #2962

Additionally I suggest adding untracked metrics to the list even though the overlay appears right. This could be useful to inform the user about metrics that could not be tested. For example every metric might be unsuspicious but what about the other metrics that have not been tested. This feature allows the user to be warned about "untracked" metrics.

Next steps:

  • Fix material tooltip (hovering does not work and alignment is not right yet)
  • Improve code readability
  • Write tests
  • Add CHANGELOG.md entry

@jannikr jannikr marked this pull request as ready for review August 30, 2022 12:44
Copy link
Contributor

@Hall-Ma Hall-Ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have another idea to this issue. All you need is this:

	for (const key in metricValuesByLanguages[mainProgrammingLanguage]) {
		const keys = Object.keys(languageSpecificMetricThresholds)
		if (!keys.includes(key) && !metricAssessmentResults.untrackedMetrics.includes(key)) {
			metricAssessmentResults.untrackedMetrics.push(key)
		}
	}

So we wouldn't need that array in array thing, that is really confusing me :D
But your approch is also good!

@jannikr jannikr marked this pull request as draft August 30, 2022 15:44
@jannikr
Copy link
Contributor Author

jannikr commented Aug 31, 2022

I have another idea to this issue. All you need is this:

	for (const key in metricValuesByLanguages[mainProgrammingLanguage]) {
		const keys = Object.keys(languageSpecificMetricThresholds)
		if (!keys.includes(key) && !metricAssessmentResults.untrackedMetrics.includes(key)) {
			metricAssessmentResults.untrackedMetrics.push(key)
		}
	}

So we wouldn't need that array in array thing, that is really confusing me :D But your approch is also good!

@Hall-Ma I agree with you and find your solution more feasible!

@jannikr jannikr marked this pull request as ready for review August 31, 2022 06:30
@sonarcloud
Copy link

sonarcloud bot commented Aug 31, 2022

[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Aug 31, 2022

[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@jannikr jannikr merged commit 4f1d8fd into main Aug 31, 2022
@jannikr jannikr deleted the fix/2962/empty-suspicious-metrics-overlay branch August 31, 2022 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants