Create form for create and update post #32

serhii73 · 2018-06-01T10:33:34Z

No description provided.

serhii73 · 2018-06-02T14:34:53Z

@webknjaz we can merge this Django project in master?

webknjaz · 2018-06-02T17:00:46Z

No

webknjaz

Let's do rebase and commits cleanup in each PR before merging from now on.

webknjaz · 2018-06-16T09:35:01Z

blog/forms.py

+    """Create and edit form."""
+
+    class Meta:
+        """Build form on the model and with the fields."""


It's not an action.

webknjaz · 2018-06-16T09:35:53Z

blog/forms.py

+
+
+class PostForm(forms.ModelForm):
+    """Create and edit form."""


It's not an action, but entity.

@webknjaz Creating and editing forms. ? Is good doc for this class?

webknjaz · 2018-06-16T09:36:26Z

blog/templates/blog/base.html

+          href="//maxcdn.bootstrapcdn.com/bootstrap/3.2.0/css/bootstrap.min.css">
+    <link rel="stylesheet"
+          href="//maxcdn.bootstrapcdn.com/bootstrap/3.2.0/css/bootstrap-theme.min.css">
+    <link href='//fonts.googleapis.com/css?family=Lobster&subset=latin,latin-ext'


Don't mix different quotation styles.

webknjaz · 2018-06-16T09:37:41Z

blog/templates/blog/post_detail.html

+        {% endif %}
+        {% if user.is_authenticated %}
+            <a class="btn btn-default"
+               href="{% url 'post_edit' pk=post.pk %}"><span


Reformat this with the hierarchy in mind, not just do a new line on random space.

@webknjaz Sorry?

serhii73 · 2018-06-16T10:07:02Z

@webknjaz I fixed all problems.

webknjaz · 2018-06-17T09:24:41Z

blog/views.py

@@ -1,3 +1,50 @@
 """Views for the blog app."""
+from django.shortcuts import render, get_object_or_404
+from django.utils import timezone
+from .models import Post


Don't mix app imports with framework imports.

webknjaz · 2018-06-17T09:26:34Z

blog/views.py

+
+def post_list(request):
+    """Show list posts on the page."""
+    posts = Post.objects.filter(published_date__lte=timezone.now()).order_by(


Better readable:

posts = ( Post.objects. filter(published_date__lte=timezone.now()). order_by('-published_date') )

webknjaz · 2018-06-17T09:30:11Z

blog/templates/blog/post_list.html

+    {% for post in posts %}
+        <div class="post">
+            <div class="date">
+                <p>Опубликовано: {{ post.published_date }}</p>


Please replace all translatable strings with i18n ({% trans "Posted on:" %}).
This requires {% load i18n %}.

webknjaz · 2018-06-17T09:30:56Z

blog/templates/blog/post_detail.html

+        {% if user.is_authenticated %}
+            <a class="btn btn-default"
+               href="{% url 'post_edit' pk=post.pk %}"><span
+                    class="glyphicon glyphicon-pencil"></span></a>


make this nicer

webknjaz · 2018-06-17T09:31:41Z

blog/templates/blog/base.html

+<div class="page-header">
+    {% if user.is_authenticated %}
+        <a href="{% url 'post_new' %}" class="top-menu"><span
+                class="glyphicon glyphicon-plus"></span></a>


this kind of nesting is unreadable

webknjaz

And add pyflakes to pre-commit.

serhii73 · 2018-06-17T20:05:29Z

@webknjaz Sorry,

pyflakes - This hook runs pyflakes. (This is deprecated, use flake8).

from pre-commit.com/hooks.html
If I right to understand it says here that I need to use flake8 and my code has it.

webknjaz · 2018-06-17T20:26:06Z

It uses only a limited subset of pyflakes. try pylint then

webknjaz

use i18n in all other places.

serhii73 · 2018-06-18T09:21:58Z

@webknjaz

It uses only a limited subset of pyflakes. try pylint then

I need to create a new branch or add pylint to pre-commit here?

webknjaz · 2018-06-18T09:23:12Z

Let's do it here

webknjaz · 2018-06-19T08:26:55Z

blog/templates/blog/base.html

@@ -1,4 +1,7 @@
 {% load staticfiles %}
+
+{% load i18n %}


Wrap all strings.

serhii73 · 2018-06-19T09:13:03Z

************* Module blog.admin
E:  4, 0: Unable to import 'django.contrib' (import-error)
************* Module blog.apps
E:  2, 0: Unable to import 'django.apps' (import-error)
R:  5, 0: Too few public methods (0/2) (too-few-public-methods)
************* Module blog.forms
E:  2, 0: Unable to import 'django' (import-error)
R:  9, 4: Too few public methods (0/2) (too-few-public-methods)
R:  6, 0: Too few public methods (0/2) (too-few-public-methods)
************* Module blog.migrations.0001_initial
C: 30, 0: Wrong continued indentation (remove 3 spaces).
                    models.DateTimeField(blank=True, null=True)),
                 |  ^ (bad-continuation)
C:  1, 0: Module name "0001_initial" doesn't conform to snake_case naming style (invalid-name)
C:  1, 0: Missing module docstring (missing-docstring)
E:  3, 0: Unable to import 'django.conf' (import-error)
E:  4, 0: Unable to import 'django.db' (import-error)
E:  5, 0: Unable to import 'django.db.models.deletion' (import-error)
E:  6, 0: Unable to import 'django.utils.timezone' (import-error)
C:  9, 0: Missing class docstring (missing-docstring)
R:  9, 0: Too few public methods (0/2) (too-few-public-methods)
************* Module blog.models
C: 13, 0: Wrong hanging indentation (remove 4 spaces).
            default=timezone.now)
        |   ^ (bad-continuation)
C: 15, 0: Wrong hanging indentation (remove 4 spaces).
            blank=True, null=True)
        |   ^ (bad-continuation)
E:  2, 0: Unable to import 'django.db' (import-error)
E:  3, 0: Unable to import 'django.utils' (import-error)
************* Module blog.urls
E:  2, 0: Unable to import 'django.urls' (import-error)
C:  6, 0: Constant name "urlpatterns" doesn't conform to UPPER_CASE naming style (invalid-name)
************* Module blog.views
E:  2, 0: Unable to import 'django.shortcuts' (import-error)
E:  3, 0: Unable to import 'django.utils' (import-error)
C: 19, 0: Argument name "pk" doesn't conform to snake_case naming style (invalid-name)
C: 40, 0: Argument name "pk" doesn't conform to snake_case naming style (invalid-name)
************* Module mysite.urls
E:  2, 0: Unable to import 'django.contrib' (import-error)
E:  3, 0: Unable to import 'django.urls' (import-error)
C:  5, 0: Constant name "urlpatterns" doesn't conform to UPPER_CASE naming style (invalid-name)
************* Module mysite.wsgi
E: 12, 0: Unable to import 'django.core.wsgi' (import-error)
C: 16, 0: Constant name "application" doesn't conform to UPPER_CASE naming style (invalid-name)
-----------------------------------
Your code has been rated at 1.35/10
No config file found, using default configuration

pre-commit/pre-commit-hooks#157 - Do I need to use a local hook? - https://github.com/Yelp/venv-update/blob/de9e1befdecf448cf02ceb5c57a30bcc88a58c06/.pre-commit-config.yaml#L30-L36
Or maybe to add other hooks instead pylint? - https://pre-commit.com/hooks.html

serhii73 · 2018-06-20T18:04:48Z

@webknjaz Hi. What do you say about the code? Good now or still bad?
And what I need to do with pylint? Maybe add instead another checking instrument?
Because pylint not good working.

webknjaz · 2018-06-25T12:11:49Z

.travis.yml

+before_install:
+- pip install -U pre-commit
+- pip install pylint-django==0.11.1
+- pip install -r requirements.txt


Why did you split just one requirement into a file, while listing others outside of it? Be consistent.

webknjaz · 2018-06-25T12:15:36Z

clean-up your commits

serhii73 · 2018-06-25T17:19:12Z

@webknjaz Now is good?

webknjaz · 2018-06-26T19:13:23Z

.pre-commit-config.yaml

+        "".join([os.getenv("HOME"), "/virtualenv/python", platform.python_version(),
+        "/lib/python", ".".join(platform.python_version_tuple()[:2]), "/site-packages"])
+        if os.getenv("TRAVIS") else
+        "/home/sergey/.virtualenv/django_girls/lib/python3.6/site-packages".format(os.getcwd())


NEVER hardcode paths. Also, you inject current working directory from os.getcwd() to nowhere.

webknjaz · 2018-06-26T19:16:15Z

blog/templates/blog/post_edit.html

+{% load i18n %}
+
+{% block content %}
+    <h1>New post</h1>


webknjaz · 2018-06-26T19:22:27Z

blog/views.py

+from .models import Post
+
+
+def post_list(request):


This is unrelated to the PR purpose. You need to merge #25/#24 first.
To simplify things, move pylint integration commit into a separate PR and let's merge it before other things.

serhii73 · 2018-06-27T09:24:16Z

@webknjaz done.

webknjaz

not done.

serhii73 · 2018-07-06T18:03:37Z

@webknjaz Done.

webknjaz

fix docstrings

webknjaz · 2018-07-10T21:47:27Z

blog/forms.py

+    """Class form based on the model."""
+
+    class Meta:
+        """Description metadata."""


which description?

webknjaz · 2018-07-10T21:48:01Z

blog/forms.py

+
+
+class PostForm(forms.ModelForm):
+    """Class form based on the model."""


i see that it's a class, it's stated one line above. anything useful you want to tell me with this?

webknjaz · 2018-07-10T21:48:33Z

blog/forms.py

@@ -0,0 +1,14 @@
+"""Creating and editing forms."""


Is it a prose or a module description? It's not an action, it's a module.

webknjaz · 2018-07-10T21:51:47Z

mysite/urls.py

@@ -4,5 +4,5 @@

 urlpatterns = [
    path('admin/', admin.site.urls),
-    path(r'', include('blog.urls')),
+    path('', include('blog.urls')),


How is this change related to this PR?

serhii73 · 2018-07-11T09:15:24Z

@webknjaz what do you think now about the code?

webknjaz · 2018-07-11T09:20:11Z

I think that you need to read all comments, not just ones you like.

webknjaz

Fix comments

webknjaz · 2018-07-11T09:21:03Z

blog/forms.py

@@ -0,0 +1,14 @@
+"""The module has forms who saved in DB and will be shown in HTML file."""


Mind your grammar

webknjaz · 2018-07-11T09:21:40Z

blog/forms.py

+
+
+class PostForm(forms.ModelForm):
+    """Form for our Post model."""


"our"? whose?

webknjaz · 2018-07-11T09:22:08Z

blog/forms.py

+    """Form for our Post model."""
+
+    class Meta:
+        """Description Post's metadata."""


Docstring is a description, it's obvious.

webknjaz · 2018-07-11T17:47:41Z

blog/forms.py

+    """Form for Post model."""
+
+    class Meta:
+        """Configuration for form."""


webknjaz · 2018-07-11T17:47:59Z

blog/forms.py

+
+
+class PostForm(forms.ModelForm):
+    """Form for Post model."""


Forget about model, it's about visual effect.

webknjaz

same

webknjaz

Now, fix the last comment left.

webknjaz previously approved these changes Jun 15, 2018

View reviewed changes

serhii73 dismissed webknjaz’s stale review via 2770e75 June 16, 2018 09:18

webknjaz requested changes Jun 16, 2018

View reviewed changes

webknjaz reviewed Jun 16, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch 4 times, most recently from f05021a to ddb151f Compare June 17, 2018 09:21

webknjaz reviewed Jun 17, 2018

View reviewed changes

webknjaz requested changes Jun 17, 2018

View reviewed changes

webknjaz reviewed Jun 19, 2018

View reviewed changes

blog/templates/blog/base.html Outdated

@@ -1,4 +1,7 @@

{% load staticfiles %}

{% load i18n %}

Copy link

Member

webknjaz Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap all strings.

serhii73 force-pushed the feature/6/django_forms branch 2 times, most recently from 7aab1c1 to 564a3b3 Compare June 19, 2018 11:40

webknjaz reviewed Jun 25, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from c5bec2c to cd22a76 Compare June 25, 2018 15:23

webknjaz reviewed Jun 26, 2018

View reviewed changes

blog/templates/blog/post_edit.html Outdated

{% load i18n %}

{% block content %}

<h1>New post</h1>

Copy link

Member

webknjaz Jun 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i18n.

webknjaz reviewed Jun 26, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from cd22a76 to 3dfa0a5 Compare June 27, 2018 09:20

webknjaz requested changes Jun 27, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from b7a9472 to 85f8f16 Compare June 28, 2018 10:44

serhii73 force-pushed the feature/6/django_forms branch 2 times, most recently from 4b1a716 to 2eeb8fb Compare July 6, 2018 18:03

webknjaz requested changes Jul 10, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from 2eeb8fb to 32494dd Compare July 11, 2018 09:14

webknjaz requested changes Jul 11, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from 32494dd to acdd1fb Compare July 11, 2018 16:28

webknjaz reviewed Jul 11, 2018

View reviewed changes

blog/forms.py Outdated

"""Form for Post model."""

class Meta:

"""Configuration for form."""

Copy link

Member

webknjaz Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of

webknjaz reviewed Jul 11, 2018

View reviewed changes

blog/forms.py Outdated

class PostForm(forms.ModelForm):

"""Form for Post model."""

Copy link

Member

webknjaz Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget about model, it's about visual effect.

webknjaz requested changes Jul 11, 2018

View reviewed changes

serhii73 force-pushed the feature/6/django_forms branch from b4492f1 to 1c7207c Compare July 12, 2018 13:55

webknjaz requested changes Jul 12, 2018

View reviewed changes

Create form for create and update post

74ea5eb

serhii73 force-pushed the feature/6/django_forms branch 2 times, most recently from 7c48045 to 74ea5eb Compare July 12, 2018 16:49

webknjaz approved these changes Jul 12, 2018

View reviewed changes

webknjaz merged commit 2370d29 into master Jul 12, 2018



		class PostForm(forms.ModelForm):
		"""Class form based on the model."""

		@@ -0,0 +1,14 @@
		"""The module has forms who saved in DB and will be shown in HTML file."""



		class PostForm(forms.ModelForm):
		"""Form for our Post model."""

Create form for create and update post #32

Create form for create and update post #32

Conversation

serhii73 commented Jun 1, 2018

serhii73 commented Jun 2, 2018

webknjaz commented Jun 2, 2018

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii73 commented Jun 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz left a comment

Choose a reason for hiding this comment

serhii73 commented Jun 17, 2018

webknjaz commented Jun 17, 2018

webknjaz left a comment

Choose a reason for hiding this comment

serhii73 commented Jun 18, 2018

webknjaz commented Jun 18, 2018

Choose a reason for hiding this comment

serhii73 commented Jun 19, 2018

serhii73 commented Jun 20, 2018

Choose a reason for hiding this comment

webknjaz commented Jun 25, 2018

serhii73 commented Jun 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii73 commented Jun 27, 2018

webknjaz left a comment

Choose a reason for hiding this comment

serhii73 commented Jul 6, 2018

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii73 commented Jul 11, 2018

webknjaz commented Jul 11, 2018

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz left a comment

Choose a reason for hiding this comment

webknjaz left a comment

Choose a reason for hiding this comment