kpi-web-guild / django-girls-blog-schenkh

django-girls-blog-schenkh created by GitHub Classroom
MIT License
0 stars 0 forks source link

Proceed with the Django Girls tutorial #21

Open webknjaz opened 5 years ago

webknjaz commented 5 years ago

Do a PR per each tutorial page. For simplicity of tracking progress, you may add markdown checkboxed list here. Do your best to apply what you learned in the previous tasks.

schenkh commented 5 years ago

@webknjaz This is the tutorial page you're talking about, right?

webknjaz commented 5 years ago

Yep. Also, note that you may be tempted to use a translated version of that, but it's usually outdated. I'd recommend sticking to the :gb: one and looking up :ukraine: only as an occasional helper. Your task here is to master feature-branch based workflow with reviews via PRs. Keep PRs scoped to only one topic. Sometimes you'll be able to work on a few different branches having different PRs and switching back and forth in order to fix things there up until they are completely polished and ready for merge.

schenkh commented 5 years ago

Hey @webknjaz I'm getting bunch of linter, compliance and import errors when trying to commit first django files. Should I try to fix them all?

schenkh commented 5 years ago
hookid: pydocstyle
blog/urls.py:1 at module level:
D400: First line should end with a period (not 'n')
blog/urls.py:1 at module level:
D413: Missing blank line after last section ('Examples')
blog/__init__.py:1 at module level:
D104: Missing docstring in public package
manage.py:1 at module level:
D100: Missing docstring in public module

hookid: mypy
blog/settings.py:28: error: Need type annotation for 'ALLOWED_HOSTS'

hookid: pylint
 ************* Module blog.urls
blog/urls.py:16:0: E0401: Unable to import 'django.contrib' (import-error)
blog/urls.py:17:0: E0401: Unable to import 'django.urls' (import-error)
blog/urls.py:19:0: C0103: Constant name "urlpatterns" doesn't conform to UPPER_CASE naming style (invalid-name)

 ************* Module blog.wsgi
blog/wsgi.py:12:0: E0401: Unable to import 'django.core.wsgi' (import-error)
blog/wsgi.py:16:0: C0103: Constant name "application" doesn't conform to UPPER_CASE naming style (invalid-name)

************* Module manage
manage.py:1:0: C0111: Missing module docstring (missing-docstring)
webknjaz commented 5 years ago

Yes, that's the point: you shouldn't have linter errors. For some things, which are enforced by the framework out of your control (ALLOWED_HOSTS), you can add a comment for linter to ignore a specific rule in that place. But most of them must be fixed. Import errors in your snippet happen because you don't have django in the same env where pylint runs, this should be fixed. For docstrings, exercise PEP 257 principles to match the style.

schenkh commented 5 years ago

I'm having hard time fixing import errors from pylint as I cannot find a way how to make it "look" on virtual env. So I followed the tutorial and created my venv folder "python3 -m venv myvenv" in my working directory and then installed django via requirements file (requirements.txt). Now I have my venv activated (source myvenv/bin/activate) and can run "python manage.py runserver 0:8000" and it works fine. What I cannot figure out is how to configure pylint to work with virtual env. As far I understand it lives in global site-packages (https://cl.ly/d624e1f957ab).. https://cl.ly/5d2acac67e65
and I need to run this version? https://cl.ly/13a32dba5570 or do something else? (https://cl.ly/4cb15b54d85f)

webknjaz commented 5 years ago

pylint is managed by pre-commit. Normally, it creates separate hooks for virtualenvs under ~/.cache/pre-commit. So it's here. Isolated.

You were instructed to use "local" mode for this one @ #7 but didn't do this. If you did it as asked, you'd have pre-commit using the current env where you'd have your extra packages installed.

Here's another dirty hack you could do: https://github.com/kpi-web-guild/django-girls-blog-serhii73/blob/master/.pre-commit-config.yaml#L30

And you can add up pylint-django on top, which changes some rules to fit Django-enforced style.

webknjaz commented 5 years ago

The local mode looks like this: https://github.com/sanitizers/octomachinery/blob/93899d4/.pre-commit-config.yaml#L51-L58. But with this, you need to install the linter manually. (I usually wrap that with tox but we'll skip learning yet another tool for now)

schenkh commented 5 years ago

Thanks, I'll need to spend some time to figure out how this stuff works cuz I'm utterly confused.

Dirty hack gives me the following error on commit attempt:

Traceback (most recent call last):
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/bin/pylint", line 10, in <module>
    sys.exit(run_pylint())
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/__init__.py", line 20, in run_pylint
    Run(sys.argv[1:])
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/lint.py", line 1536, in __init__
    linter.load_plugin_modules(self._plugins)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/lint.py", line 636, in load_plugin_modules
    module = modutils.load_module_from_name(modname)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/astroid/modutils.py", line 202, in load_module_from_name
    return load_module_from_modpath(dotted_name.split("."), path, use_sys)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/astroid/modutils.py", line 244, in load_module_from_modpath
    mp_file, mp_filename, mp_desc = imp.find_module(part, path)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/imp.py", line 296, in find_module
    raise ImportError(_ERR_MSG.format(name), name=name)
ImportError: No module named 'pylint_django'
Traceback (most recent call last):
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/bin/pylint", line 10, in <module>
    sys.exit(run_pylint())
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/__init__.py", line 20, in run_pylint
    Run(sys.argv[1:])
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/lint.py", line 1536, in __init__
    linter.load_plugin_modules(self._plugins)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/pylint/lint.py", line 636, in load_plugin_modules
    module = modutils.load_module_from_name(modname)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/astroid/modutils.py", line 202, in load_module_from_name
    return load_module_from_modpath(dotted_name.split("."), path, use_sys)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/site-packages/astroid/modutils.py", line 244, in load_module_from_modpath
    mp_file, mp_filename, mp_desc = imp.find_module(part, path)
  File "/Users/hams/.cache/pre-commit/repohhaqilmy/py_env-python3.7/lib/python3.7/imp.py", line 296, in find_module
    raise ImportError(_ERR_MSG.format(name), name=name)
ImportError: No module named 'pylint_django'
webknjaz commented 5 years ago

Well, as I mentioned before, that hack uses an extra package called pylint-django which you forgot to install.

schenkh commented 5 years ago

I did not "forget" to install it. it Based on what you said it looked like next step.

webknjaz commented 5 years ago

I'd not go for such hack, just do what I did in the octomachinery repo, it seems to be what the author of pre-commit uses himself. But yes, I'd also integrate that pylint plugin later anyway.

webknjaz commented 5 years ago

It's better if you submit a Draft/WIP PR so that it'd be visible where you are now.

schenkh commented 5 years ago

Sure, I've submitted #22. Catching up as I was not able to work on this for some time.

webknjaz commented 5 years ago

Edit that checkboxes list adding this link against the appropriate task in brackets.

schenkh commented 5 years ago

@webknjaz Im done, thanks

schenkh commented 5 years ago

@webknjaz please delete this repo if you don't mind

webknjaz commented 5 years ago

I'll archive it.