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

Fix incorrect count of merged hhc signals #2027

Merged

Conversation

vasily-pozdnyakov
Copy link
Contributor

@vasily-pozdnyakov vasily-pozdnyakov commented Sep 21, 2023

Summary of changes

Instead of summarizing of all the similar hhc signals, now we just take the max value. I decided to go easy way (not adding a lot of extra logic of deduplication of resources) as the metrics is not the most important and accurate anyway.

Context and reason for change

For merged hhc signals (where only comments are different ) there is incorrect count of signals (just a sum of all of them) which is not realistic.

How can the changes be tested

Run tests.
Open any project with merged signals and see that the number is more real now.

Note: Please review the guidelines for contributing to this repository.

Copy link
Member

@antonbauhofer antonbauhofer left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Nice catch!

Now we take max value instead of sum

Signed-off-by: Vasily Pozdnyakov <vasily.pozdnyakov@tngtech.com>
@vasily-pozdnyakov
Copy link
Contributor Author

simplified the logic

@vasily-pozdnyakov vasily-pozdnyakov merged commit a1ea30a into opossum-tool:main Sep 21, 2023
@vasily-pozdnyakov vasily-pozdnyakov deleted the fix_hhc_signals_count branch September 21, 2023 13:48
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.

2 participants