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

Feature/attribute identifier respect key #735

Merged
merged 2 commits into from
Jun 8, 2020

Conversation

diba1013
Copy link
Contributor

@diba1013 diba1013 commented Jun 8, 2020

Before submission, please check that ...

  • the code follows the Clean Code guidelines.
  • the necessary tests are either created or updated.
  • all status checks (Travis CI, SonarCloud, etc.) pass.
  • your commit history is cleaned up.
  • you updated the changelog.
  • you updated necessary documentation within retest/docs.

Description

TL;DR This fixes that the GlobalChangeSetApplier accepts similar attribute differences.

For the return of one-click-maintenance (more details in #726), the GlobalChangeSetApplier
uses the AttributeDifference#identifier() to find equal differences, since AttributeDifference#equals per definition cannot be used since it would check the warnings too.

However, the identifier currently only takes the expected and actual values into account, ignoring the key attribute. That means, following differences are considered equal, although differing in keys:

border-bottom-color: expected=rgba(0, 0, 0, 0.26), actual=rgba(0, 0, 0, 0.32)
border-top-color: expected=rgba(0, 0, 0, 0.26), actual=rgba(0, 0, 0, 0.32)

This causes review to accept these attributes with a single click (which, currently, is not the expected behavior, as that could group completely different differences).

Technically, the above example is the same difference, because in the report the differences could be grouped to border-color: expected=rgba(0, 0, 0, 0.26), actual=rgba(0, 0, 0, 0.32), but since we cannot implement grouping (technically), this is unwanted behavior.

State of this PR

I do not know what the intentions were to exclude the key from the identifier (since I could not search through the history of the different repositories), but a quick usage search indicates that this should not have any breaking side-effects.

diba1013 added 2 commits June 8, 2020 15:59
which is used within the GlobalChangeSetApplier to identify equal differences
@diba1013 diba1013 requested a review from roesslerj June 8, 2020 15:04
@diba1013 diba1013 self-assigned this Jun 8, 2020
@cla-bot cla-bot bot added the cla-signed label Jun 8, 2020
@mergify mergify bot merged commit c23252d into master Jun 8, 2020
@mergify mergify bot deleted the feature/attribute-identifier-respect-key branch June 8, 2020 16:05
@diba1013 diba1013 mentioned this pull request Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants