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/2789/delta mode with codecharta #3016

Closed
wants to merge 14 commits into from

Conversation

Hall-Ma
Copy link
Contributor

@Hall-Ma Hall-Ma commented Aug 31, 2022

Activate delta mode when roots are the same but have different paths

Closes: #2789

Description

Screenshots or gifs

Copy link
Contributor

@MW-Friedrich MW-Friedrich left a comment

Choose a reason for hiding this comment

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

I've had the chance to talk with @knoffi about the changes! The new code looks great and is probably more in line with what we need (YAGNI and all), but we still had a few, small pointers.

  • Rename compareCollapsedRoot to better fit the function name
  • Rename haveSameRoots, so it's clearer that only the name is compared
  • Think of what should happen if two files can not be compared (doe to a malformed/weird cc.json) Maybe we can/should display some sort of toast or notification for the user?

But it's already a definite LGTM from us both!

@Hall-Ma
Copy link
Contributor Author

Hall-Ma commented Sep 1, 2022

I'm not really happy with this solution. It's working but I don't like that I also have to handle this root thing in codecharta service (unsubscribeReferenceFileSubscription). Unfurtanetly this service isn't migrated yet, perhaps we can deal with it later in a separate selector.
Another solution I can see is that we could create a root folder (first slash) and push everything as a child (after the second slash) so that we could gain the 'original' or rather same root sequence as the compared file. But is it worth the effort?

@Hall-Ma Hall-Ma requested review from ce-bo and removed request for BridgeAR September 6, 2022 11:29
@Hall-Ma
Copy link
Contributor Author

Hall-Ma commented Sep 14, 2022

@shaman-apprentice Maybe you can keep an eye on that?
I had to introduce a comparison file selector to update the root name for the comparison file. Overall this works, but when I have different root names like root/ (for reference file ) and root/app (for comparison file) the switch button in delta mode doesn't work anymore or work not properly. I get an empty map. :(
But actually I would like to compare these two maps anyway, because they have the same root name (in principle). I could send you both cards or maybe we have enough time on Friday.
On Commit 2621f43 it worked but I wanted to implement it in the best way :D

EDIT: @shaman-apprentice Please ignore my comment for now.

@sonarcloud
Copy link

sonarcloud bot commented Sep 14, 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 Sep 14, 2022

[CodeCharta Visualization] SonarCloud Quality Gate failed.    Quality Gate failed

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

71.9% 71.9% Coverage
0.0% 0.0% Duplication

@ce-bo
Copy link
Collaborator

ce-bo commented Sep 19, 2022

This issue will not be fixed. This bug is produced by comparing maps with different structures. So it is fine, that the two compared maps are not shown in delta mode, even if they are very similar. It would take to much effort to refactor the code parts. So I close this for now.

@ce-bo ce-bo closed this Sep 19, 2022
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.

Delta Mode not working for codecharta.cc.json
4 participants