Skip to content

Fix fingerprint bug - don't use num occurrences #92

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 29, 2016

Conversation

ABaldwinHunter
Copy link
Contributor

Using the number of occurrences of duplicated code as a unique
identifier of the issue makes the fingerprint non-deterministic
with respect to a given issue at a site of the duplicated code.

Opting to use checkname instead.

Fixes false positives in PR comparisons: https://codeclimate.com/repos/569fa9e70c40f068790031e2/pull/3#duplication

cc @codeclimate/review

Using the number of occurrences of duplicated code as a unique
identifier of the issue makes the fingerprint non-deterministic
with respect to a given issue at a site of the duplicated code.

Opting to use checkname instead.
@gdiggs
Copy link
Contributor

gdiggs commented Jan 29, 2016

LGTM

ABaldwinHunter pushed a commit that referenced this pull request Jan 29, 2016
Fix fingerprint bug - don't use num occurrences
@ABaldwinHunter ABaldwinHunter merged commit dfdc557 into master Jan 29, 2016
@ABaldwinHunter ABaldwinHunter deleted the abh-fingerprints branch January 29, 2016 17:16
@@ -86,7 +86,7 @@ def fingerprint
digest << "-"
digest << current_sexp.mass.to_s
digest << "-"
digest << occurrences.to_s
digest << check_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Classic also uses the type of the sexp's root node, I think. That's an extra piece of info to make the fingerprint more unique that may be worth adding.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants