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

Semantic diffs aren't very helpful for us #435

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Semantic diffs aren't very helpful for us #435

merged 1 commit into from
Jun 28, 2023

Conversation

therealryan
Copy link
Collaborator

We use diff-match-patch in the report app for three things:

  • The text diff component
  • The coverage indicator
  • The model change analysis

For the first of these usages we saw that the diff_cleanupSemantic wasn't actually very helpful for the sort of text that we were working with, so we left it out.

That method was already being called for the other two usages - this change removes those calls so that all diff operations are consistent.

In the process of this change we noticed that:

  • Chrome is no longer putting a startup notice on stderr - we can remove our own shutdown notice
  • Click from the model change analysis page to a change didn't work as expected - the diff component wasn't being refreshed correctly.

@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2023

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
0.0% 0.0% Duplication

@therealryan therealryan merged commit 5d06f74 into main Jun 28, 2023
10 checks passed
@therealryan therealryan deleted the raw_diffs branch June 28, 2023 09:27
@therealryan therealryan added the enhancement New feature or request label Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant