-
Notifications
You must be signed in to change notification settings - Fork 189
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
Implement --diff-range-notation option #108
Conversation
Looks good to me. I left a question and curious on your thinking. But if you feel strongly about leaving it as is im happy to leave it and merge |
addCleanup is considered more resilient and safer because tearDown methods are not executed in case the setUp method fails. They also bring resource creation and deletion closer together, facilitating refactorings.
After you are happy with this changes and this gets merged, could we have another release? I'm happy to open the PR preparing the release if that helps. I don't see instructions on how to make a release, but from what I can gather:
|
Woot! Thanks for all the work on this. Ill handle getting the release out. let me look at one other PR and get this done tonight |
Awesome, thanks! |
just crossing my fingers hoping my recently added 2fa did not break the travis CD pipeline.... |
Thanks! |
I've also used
self.addCleanup
to undo setup changes in a few places, after noticing that the patches intest_git_diff.py
were not being undone.addCleanup
is considered more resilient and safer becausetearDown
methods are not executed in case thesetUp
method fails, which might leave resources around.They also bring resource creation and deletion closer together, facilitating refactorings.
Fix #105