Adjust inspectdb management command to yield output following the Django Coding Style #2173

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@JustinTArthur

With this change, the successful output of running the inspectdb command complies with style guidelines set forth in /docs/internals/contributing/writing-code/coding-style.txt. While that document refers to the best practices when coding for The Django Project, and not a developer's own project, I feel that consistency with internal practices is good.

I've added a test that checks for this behavior using the 3rd-party pep8 Python library (included).

@timgraham
Django member

I like the change, but prefer we don't bundle pep8. How about if we skip the test if pep8 isn't installed? There are other tests that do this for other libraries you can reference.

@JustinTArthur

I like that better, too. Other tests I looked at did it the bad way, so I followed suit. Pull-branch updated to realize @timgraham's suggestion.

@berkerpeksag berkerpeksag commented on an outdated diff Jan 17, 2014
tests/inspectdb/tests.py
@@ -9,6 +9,13 @@
from django.test import TestCase, skipUnlessDBFeature
from django.utils.six import PY3, StringIO
+try:
+ import pep8
+ HAS_PEP8 = True
@berkerpeksag
berkerpeksag Jan 17, 2014

The HAS_PEP8 constant is unnecessary IMO.

try:
    import pep8
except ImportError:
    pep8 = None

Then:

@unittest.skipIf(pep8 is None, "needs pep8")
def test_something(self):
    pass
@timgraham
Django member

After thinking about it a bit more, I suggest we leave out the test for this. I don't think the chance of a regression here is very large and the test is rather complex.

@JustinTArthur

Adjusted pull request commit to include only the changes to the inspectdb output and pre-existing tests of that output.

@timgraham
Django member

Thanks Justin, merged in 298a2b5.

@timgraham timgraham closed this Jan 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment