bug fix: last pr had issue if venv wasn't being utilized #56
Conversation
django_fastdev/apps.py
Outdated
| # just in case venv resides in project dir | ||
| if venv_dir and module_path.startswith(str(venv_dir)): | ||
| return False | ||
| return True |
There was a problem hiding this comment.
This new code is not what I suggested, and it's broken. It implicitly returns None.
There was a problem hiding this comment.
I'm confused, you me the if venv_dir bit? I just use implicit truthy testing in case somehow venv_dir is an empty string (using is None would cause that check to pass, and then startswith() would always succeed since it is being passed a blank string)
There was a problem hiding this comment.
No, I mean your code is like:
def foo():
if x:
return True
# NO ELSE HEREThat's the problem.
There was a problem hiding this comment.
ohhh, duh! fixed, sorry! :P
django_fastdev/apps.py
Outdated
| @@ -83,6 +83,15 @@ def get_gitignore_path(): | |||
| return None | |||
|
|
|||
|
|
|||
| def get_venv_path(): | |||
| return os.environ.get("VIRTUAL_ENV", None) | |||
There was a problem hiding this comment.
good point! I did a little bit of research to reliably determine the best ways to get the venv path, and chained them all into the get_venv_path() func now. should be hard to find a scenario where it won't pick up now I think
I am sorry my friend! I found a bug in my code from my previous pr. if one wasn't using a venv (and using python at root level instead), the check against project dir would always fail. additionally, I made a mistake in the if statement that checked whether or not a form should be verified (I should have grouped the or operand). this pr fixes these errors. forgive my mistakes! :(