-
Notifications
You must be signed in to change notification settings - Fork 10
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 flag to ignore file hashes #65
Conversation
b443c14
to
f8ed3e6
Compare
rebasing on top of latest develop branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation-wise this is good to go.
However it still requires UI work.
Its important to mark debug output as such.
Since you do need to reuse it a bunch of times, it might be worth defining a new constant or write a new helper function in util.py
.
Also make sure that the code stays readable.
It might be clear to you that the hash_dict
is gonna be filled with hash_entries
which then again are filled with a type
and a hash
, but hiding all that behind a single letter is still not ideal.
No major changes required though. All minor UI stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main points:
- Should an end-user have access to this?
I see two options:- make it invisible in the help
- adjust the description to make clear its potentialy dangerous
- Make sure to use the new
print_debug()
in all places where manual debug styling was done before - Dont print EVERYTHING on the terminal "just because"
- be aware of all possible situations
obtain_source
can be called in.
Make sure to not only test the changes on the system you had the issue on, but also on one where it never occured/ you had the files already/ the files are actually faulty/ ...
Especially here in obtain_source
, a lot of different states and edgecases can happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Add a flag to ignore invalid file hashes.
When both
--debug
and--ignore-hash
flags are set, invalid checksums will produce warnings but the test will continue.