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
Refactor settings #449
Refactor settings #449
Conversation
It only gets called from inside another private method anyhow
to reduce cognitive load
On my local machine the install test needs
For now I just wrapped the removal in a |
if __name__ == "__main__": | ||
unittest.main() |
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.
Why is this part removed? I though we need it to run the tests in the continuous integration environment.
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.
That's a very good warning!!! It's not necessary on my local machine so I thought I was removing dead code. I'll just burn some CPU minutes and do a brute-force tests by throwing a failing test in one of these suites to see if the CI catches it.
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.
Indeed, it's safe to remove the __main__
boilerplate. I added this to one of the modules that had had the boilerplate stripped and the CI caught it no problem:
def test_should_fail(self):
self.assertTrue(False)
I'm going to revert the change and resolve the other two instances, but it was a very good call to double check this behaviour!!!
Just some minor housekeeping for readability