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

Bugfix/115 bug in 2 of the detectors transmittedmtofxandyandsubregionhistdetector and reflectedmtofxandyandsubregionhistdetector #119

Conversation

hayakawa16
Copy link
Member

No description provided.

Lisa Malenfant and others added 3 commits October 11, 2023 11:42
…or and TransmittedMTOfXAndYAndSubregionHist to use Y DoubleRange different from X DoubleRange to verify code change in Binary Serialization code for these detectors. Also cleaned up code in this unit test. I renamed file ReflectedMTOfRhoAndSubRegionHist to ...SubregionHist but not showing here to commit.
@lmalenfant
Copy link
Member

@hayakawa16 the Sonar Cloud analysis failed because the duplication is 52.5% for this code, I have it set to 20% on our rule but I guess it will go over that number even after we merge (20.8%). This duplication number has been creeping up so even setting it at 20% was not high enough, should I increase it more?

@hayakawa16
Copy link
Member Author

Not sure I understand. Did the duplication increase due to my code changes? Or is it the same percentage as it was before my changes?

@hayakawa16
Copy link
Member Author

If I'm reading the analysis correctly, it is due to the renaming of the file ReflectedMTOfRhoAndSubregionHistDetector.cs. Not sure why.

@lmalenfant
Copy link
Member

lmalenfant commented Oct 26, 2023

Not sure I understand. Did the duplication increase due to my code changes? Or is it the same percentage as it was before my changes?

No it didn't but I think now that the files changed, it calculates based on "new code" so this is actually both our changes that caused new code to show 52.5%. The previous code coverage percentage was 20.8% so maybe merging this to master will not fail the analysis. I could also raise it to 21% just to pass this build.

@lmalenfant
Copy link
Member

I approved this PR so we can merge as soon as you would like, hopefully it will pass the quality gate.

…ts named correctly with capital at start already.
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

95.7% 95.7% Coverage
52.5% 52.5% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@lmalenfant
Copy link
Member

Let's merge this one and keep our fingers crossed that it passes code quality :)

@lmalenfant lmalenfant merged commit 6cf2efa into master Oct 27, 2023
@lmalenfant lmalenfant deleted the bugfix/115-bug-in-2-of-the-detectors-transmittedmtofxandyandsubregionhistdetector-and-reflectedmtofxandyandsubregionhistdetector branch October 27, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants