Review

---

go to 4477cb9

George Rymarenko 18 сент., 16:48

Константин, добрый день!

Мои поздравления с началом 4-го проекта. Это дипломная работа, обозначающая завершение целой профессии. Спустя несколько месяцев кропотливой работы мы подобрались к финишу. Впереди интересная работа по совершенствованию проекта. Приступим.

[x] Прежде всего отмечу детальный README, позволяющий получить много информации о проекте. В предыдущих проектах мы использовали asciinema, чтобы увидеть демонстрацию проекта. Здесь же такой возможности нет, но можно оставить ссылку на приложение, развернутое на Heroku.

[x] Предлагаю еще раз подумать над разделом Installation. Кажется, что в нем сейчас есть команды, дублирующие Make-файл, и информация, не имеющего непосредственного отношения к проекту (настройка переменных Heroku, в частности). Так, к примеру, клонирование репозитория и создания окружения пусть останутся на усмотрение пользователя. А вот установка зависимостей - первый шаг, который действительно необходимо сделать, и он есть в Make-файл. Подготовка .env-файла - такой же очередной шаг, пусть он будет также находится в Make-файл. Сам пример .env-файла может хранить минимум, необходимый для запуска приложения локально, это позволит сразу запустить приложение без необходимости осуществлять настройку переменных. В идеале этот раздел должен включать четкие шаги (делай раз, делай два, делай три), после которых проект запустится локально.

[x] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/.env_example#L1 Хранить пароли в репозитории не рекомендуется, даже если они тестовые. Можно и удобно заменять на говорящие значения вроде “secret_key_to_be_replaced_in_production”, “postgres://user:password@host:port/name”.

[] Теперь внимательно посмотрим на структуру проекта. На первом шаге сказано: “Проект должен содержать приложение Django с именем task_manager, содержащее модуль settings. Другими словами, проект, который мы разрабатываем, можно использовать снаружи: from task_manager import settings. Здесь все сделано в соответствии с требованиями. Если посмотреть немного шире, то становится понятно, что это Django-приложение, которое как раз и разворачивается, то есть оно должно инкапсулировать в себя все необходимое. Таким образом снаружи task_manager находится все, что относится к окружению - настройка, запуск приложения. Внутри - все, что относится к приложению. Необходимо решить, что сейчас оказалось снаружи, а должно быть внутри. Тут также следует обратить внимание, что результат команды django-admin startproject, запущенной только с названием проекта, может сбить с толку, учитывая, что у нас уже есть проект python-project-lvl4. Кажется, что необходимо наложить один проект на другой, но при этом выполнить команду на уровень выше чем python-project-lvl4 мы не можем, так как сам Django установлен внутри python-project-lvl4. Если посмотреть внимательно на документацию: https://docs.djangoproject.com/en/3.2/ref/django-admin/#startproject и https://docs.djangoproject.com/en/3.2/ref/django-admin/#startapp, то можно понять, как лучше запускать команду django-admin startproject и django-admin startapp, чтобы добиться оптимальной структуры.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/settings.py#L145 Статика, как правильно, находится совершенно в другом месте, нежели само приложение. В нашем случае следует ограничиться тем, чтобы помещать статику на том же уровне, что и приложение, но не внутри его. Сразу стоит подумать о том, чтобы сгенерированная статика не попала в репозиторий.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/settings.py#L34 Среда и дебаггинг - все же разные вещи. Когда видишь флаг DEBUG, ожидаешь, что он имеет логический тип, но не строковый.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/settings.py#L36 Такое также лучше вынести в переменные окружения.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/settings.py#L70 Лучше включать / выключать в зависимости от флага DEBUG.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/settings.py#L130 Вот тут интересный вопрос. При текущей реализации приложение всегда будет переводить на русский язык, даже когда у клиента установлен английский. Как сделать, чтобы была зависимость от языка клиента?

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L8 Интересный способ оставлять комментарии в HTML, ранее с таким не сталкивался. Я бы предпочел все же традиционный подход.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L40 Много комментированного кода усложняют восприятие другим разработчиком.

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L9 https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L12 Локаль и библиотеку следует загружать самыми первыми еще до определения <!doctype html>, как сказано в документации: https://docs.djangoproject.com/en/3.1/topics/i18n/translation/#internationalization-in-template-code. Также можно посмотреть примеры: https://github.com/zostera/django-bootstrap4/tree/main/example

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L15 https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/task_manager/templates/base.html#L16 CSS лучше разместить в разделе head. А вот c JS все интереснее) Если смотреть на документацию Bootstrap, то рекомендую ближе к концу тела: https://getbootstrap.com/docs/5.1/getting-started/introduction/#starter-template Но если смотреть документацию django-bootstrap, то JS идет бок о бок с CSS: https://django-bootstrap4.readthedocs.io/en/latest/quickstart.html#example-template

[] https://github.com/mnogom/python-project-lvl4/blob/4477cb94600b85da7faa5976f42b61c8ab76be90/label/templates/labels.html#L1 Можно провести эксперимент - назвать данный шаблон и шаблоны в других доменах index.html, таким образом у нас получится несколько шаблонов с одинаковым названием. В теории при рендеринге может произвольно браться любой из шаблонов, так как наименование совпадет. После можно еще раз почитать документацию: https://docs.djangoproject.com/en/3.2/intro/tutorial03/#write-views-that-actually-do-something, ну а после решить, как лучше разместить шаблоны.