Closed sonofmun closed 6 years ago
Issue #58 (finally)!
The reduction in coverage is due to the objects and functions around the custom_dtd test. If this is deleted, coverage should be more acceptable.
Should we have a call tomorrow or the day after to review this together ?
Sure.
Changes have been made. Take a look.
Need to add a unit test for when the --scheme
argument is a file path.
Should I wait for the unit-test for merging ?
Hi Sorry. Yes, wait for the next commit. I actually didn't even implement the local file option on --scheme
. So I am trying to do that now before I run out of energy for the day.
Good luck !
Should it be in this PR that we update the README ?
Implemented and tested. Don't merge yet. If you approve, just keep your review as approved and I will look at everything more closely tomorrow. Right now I am too tired to be able to judge very well what I did. I will then squash and merge if everything looks good in the morning.
Looks good to me. Will remain README, bump and CHANGES.md to do :)
I assume this is a minor revision and so we should bump to 1.2.
Minor but yes, 1.2.0 : https://semver.org/ :)
I said "minor" revision. Just look at my comment!
Sorry buddy, github updated their edit policy :
:D
Damn! Transparency sucks!
Now on to:
Yay !
If you are still OK with the changes, then I will squash and merge once all checks pass.
Squash and merge away !
Congrats !
It should detect and donwload if it is remote and detect and use if it is local. Also wrote unit tests for this. I would also suggest removing the "remote" and "local" flags from the command line (https://github.com/Capitains/HookTest/blob/custom_dtd/HookTest/cmd.py#L25) and the "custom_rng" scheme and test (e.g., https://github.com/Capitains/HookTest/blob/custom_dtd/HookTest/capitains_units/cts.py#L430-L460),