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

Include checksum as part of key for each file in store #128

Closed
wants to merge 1 commit into from

Conversation

nassredean
Copy link
Contributor

Fixes: #118

This PR adds a checksum key to each file in redis. The checksum is computed with with the contents of the file. Before persisting to redis, we determine if the checksum has changed. If so we overwrite the values for the given file in redis, if not we merge them.

Was speaking with @kbaum and we are thinking it makes sense to add a cache for the checksum so we don't have to compute the hash every time.

@danmayer
Copy link
Owner

danmayer commented Oct 9, 2018

OK this feature I think we should likely hold until version 3.0. Cool to work on and try out the proof of concept but likely will have some conflicts / rewriting when the 3.0 branch gets completed. That make sense @dnasseri / @kbaum

@kbaum
Copy link
Collaborator

kbaum commented Oct 9, 2018

@danmayer - I trust your judgement here but I do think this would be a big leap forward in making coverband a low maintenance library for users while improving it's trustworthiness. Currently users have to figure out a strategy for how often to clear their data for all of their source files. In a decent size team, other developers start to doubt the coverage data because they are unsure how long coverband has been gathering data. With this change, coverband users can just let coverband track forever. I was thinking we should follow up this change with added information on the report about when coverband began tracking data for this file. For example:

Coverage data for user.rb since 9/25/2018

@danmayer
Copy link
Owner

ok closing this out as the work and ideas are being pulled into #137

@danmayer danmayer closed this Nov 11, 2018
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.

3 participants