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

Add plaintext fingerprint to warning hash #1067

Closed
wants to merge 3 commits into from
Closed

Add plaintext fingerprint to warning hash #1067

wants to merge 3 commits into from

Conversation

brentjo
Copy link

@brentjo brentjo commented Jun 30, 2017

Occasionally when comparing two brakeman reports to find new warnings introduced by a code change, it will report an old warning and it's not immediately apparent why the code change caused the warning to reappear. You would assume it's because the fingerprint changed, but just seeing that two hashes are different doesn't give you any context for why it changed. This PR breaks up the one fingerprint method into two — one to generate the string and the other to hash it. This allows you to pass the plaintext fingerprint into the warning hash.

It's entirely possible this PR is just adding redundant data to the hash, and we would be fine using the data already in the hash (since it makes up most of the fingerprint), but having access to the fingerprint string would tell you with certainty why a fingerprint changed. I feel like people might find the fingerprint text useful to have, though perhaps it's not needed for most use cases.

@presidentbeef
Copy link
Owner

Hi Brent,

Thank you for the contribution.

However, I don't think adding this information to the JSON output is worth when I don't believe this is something very many users need very often.

What would be cool is if the --compare option could indicate why a fingerprint has changed...I'm not sure how hard that would be though.

@presidentbeef
Copy link
Owner

Whoops.

Alternatively, maybe this could be optional information?

Repository owner locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants