Skip to content
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

Task1 #1

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Task1 #1

wants to merge 11 commits into from

Conversation

ligzer
Copy link
Owner

@ligzer ligzer commented Dec 22, 2021

No description provided.

@@ -0,0 +1,60 @@
from django.core.management.base import BaseCommand, CommandError
from faker import Faker
import random
Copy link

Choose a reason for hiding this comment

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

По pep8 идут сначала импорты стандартных библиотек, затем установленных, а потом локальных (и все группы разделяются пробелами):
https://www.python.org/dev/peps/pep-0008/#imports

import random
from datetime import time

from django...

Copy link

Choose a reason for hiding this comment

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

Comment on lines +1 to +5
POSTGRES_USER=dbuser
POSTGRES_DB=task1
POSTGRES_PASSWORD=dbpasswd
POSTGRES_HOST=db
POSTGRES_PORT=5432

Choose a reason for hiding this comment

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

It should be .env.example, .env is meant to be in the start-ready environment.

Comment on lines +24 to +29
SECRET_KEY = 'django-insecure-_101p89468^yfmb3wl7*k@wim8k0_mx$w6t@@tq1iaz11_-&e&'

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = []

Choose a reason for hiding this comment

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

These config params should be the environment variables, especially, SECRET_KEY

Comment on lines +39 to +42
OpenTime = models.TimeField()
CloseTime = models.TimeField()
# TODO: Add check of unique DayOfWeek for one Store
DayOfWeek = models.IntegerField(validators=[MinValueValidator(0), MaxValueValidator(7)])

Choose a reason for hiding this comment

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

Class fields should be in the snake case.

Comment on lines +39 to +42
OpenTime = models.TimeField()
CloseTime = models.TimeField()
# TODO: Add check of unique DayOfWeek for one Store
DayOfWeek = models.IntegerField(validators=[MinValueValidator(0), MaxValueValidator(7)])

Choose a reason for hiding this comment

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

What about enum.Enum or django's TextChoices? It probably would be a bit more clear and readable

Comment on lines +1 to +3
from django.shortcuts import render

# Create your views here.

Choose a reason for hiding this comment

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

Omit blank files

Comment on lines +1 to +6
Django>=4.0,<5.0
psycopg2-binary
djangorestframework
django-filter
ipython
Faker

Choose a reason for hiding this comment

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

pip freeze requirements.txt would set specific versions of your environment automatically for your requirements file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants