-
Notifications
You must be signed in to change notification settings - Fork 284
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
tests: drop dependency on external mock module for newer python #574
Conversation
Starting python 3.3, this is in the stdlib. Fixes html5lib#541
Please rebase so the tests pass. |
All things considered I'd rather wait until I have core maintainer review and then rebase once instead of potentially many times. |
If you were a core maintainer, would you feel more comfortable about reviewing a PR with failing tests (current state) or passing tests? https://ci.appveyor.com/project/gsnedders/html5lib-python/history |
html5lib/tests/test_meta.py
Outdated
from mock import Mock | ||
try: | ||
from unittest.mock import Mock | ||
except: |
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.
except: | |
except ImportError: |
See PEP8 or
If I were a maintainer, and:
I would not appreciate external drive-by contributors pressuring other external contributors to feel guilty about my project which doesn't have passing tests in the first place. But also, as it happens, yes, I absolutely would review PRs with failing tests! Even if the failing test is a flaw in the contribution itself. I do this all the time. Then I offer suggestions to the contributors, who may not be fully aware of deeply hidden quirks of the codebase that require specialized knowledge to handle correctly, knowledge which I can provide in order to aid those contributors in getting their PR into a mergeable state. Sometimes I, gasp, merge PRs with failing CI because the PR fixes one CI error but not all the errors, and CI will never be fixed if you're not willing to fix issues one by one and instead expect it all to be fixed in one fell swoop and the task becomes so intimidating that everyone just gives up and it never gets fixed. Sometimes I review a PR and say "lgtm as soon as CI passes, please rebase", so that contributors can feel confident that they're not being ignored, and that if they rebase the PR now then they won't be asked to rebase it once a month for the next year only to have it never merged. In short, yes, I review PRs without regard to whether they have passing CI, because I do not see what passing CI has to do with anything at the review stage. |
Starting python 3.3, this is in the stdlib.
Fixes #541