stgolovin / welbex

0 stars 0 forks source link

Code review #1

Open Lagger0k opened 1 year ago

Lagger0k commented 1 year ago

Привет. Увидел твое резюме в группе ТГ, зашел поглядеть, с вашего позволения оставлю небольшое ревью по проекту, если указывать ссылку на github в резюме, то проекты тут должны быть сильно выше уровнем, можно попробовать улучшить следующие вещи.

  1. pycache ну ни как не должны быть в проекте, добавьте их в .gitignor
  2. tests.py если не пишите тесты, то удаляйте автосгенерированные файлы (но лучше писать)
  3. views.py если используете DRF, то зачем плодить view на каждый тип запроса? Можно сделать примерно так
    
    from rest_framework import mixins
    from rest_framework.viewsets import GenericViewSet

class CargoViewSet(mixins.RetrievModelMixin, mixins.UpdateModelMixin, mixins.CreateModelMixin, GenericViewSet) """Карго вьюха"""

def get_queryset(self, request):
    qs = Cargo.objects.all()
    return qs

def get_serializer_class(self, request):
    match self.action:
        case 'retriev':
            return RetrieveCargoSerializer
        case 'post':
            return CreateCargoSerializer

ну и так далее, можно все в 1 вью уместить.
4. [тут](https://github.com/stgolovin/welbex/blob/557da29d6d7c88c0df77dd2795f486905f5eadc5/cargo/serializers.py#L12) лучше использовать tuple
5. [fildb.py](https://github.com/stgolovin/welbex/blob/main/welbex/fildb.py) из названия вообще не понятно что делает этот файл, а внутри половина кода закомментирована, если код закомментирован, его надо удалять
6. Если пишите docker-compose для Django приложения, то наверное нужно его запускать в связке с Nginx, и уж точно не через runserver
7. Базу данных можно накачать данными локально, потом снять дамп, а в docker-compose поднять postgres уже с дампом, удобнее проверяющему будет
8. Для применения миграций не нужно делать отдельный контейнер, можно написать  entrypoint.sh на bash и стартовать основное приложение через него
9. не видно работы с .env переменными (хотя бы django_secret_key туда нужно убрать)
10. Код не отформатирован, рекомендую использовать black (есть удобные плагины на pycharm или vscode)
11. [тут](https://github.com/stgolovin/welbex/blob/557da29d6d7c88c0df77dd2795f486905f5eadc5/welbex/settings.py#L28) не должно быть пусто
12. ридми должно быть более информативно, хотябы с инструкцией как запустить проект.
13. Было бы не плохо добавить везде type хинты

Это все, что сразу бросилось в глаза, на самом деле, будь я нанимающим, закрыл бы через 2 мин, если указываете GH в резюме, то нужно основательно поработать над последними проектами и тогда будет уже что обсудить на собесе.
Успехов.
stgolovin commented 1 year ago

Приветствую, Максим! Благодарю за ревью!! Внесу изменения и исправления) не все правда мне пока понятно как сделать, но это поправимо) Как например поднять базу Postgres в докер уже с данными? Sent from my iPhoneOn 23 Jun 2023, at 00:29, Maksim Myakotin @.***> wrote: Привет. Увидел твое резюме в группе ТГ, зашел поглядеть, с вашего позволения оставлю небольшое ревью по проекту, если указывать ссылку на github в резюме, то проекты тут должны быть сильно выше уровнем, можно попробовать улучшить следующие вещи.

pycache ну ни как не должны быть в проекте, добавьте их в .gitignor tests.py если не пишите тесты, то удаляйте автосгенерированные файлы (но лучше писать) views.py если используете DRF, то зачем плодить view на каждый тип запроса? Можно сделать примерно так

from rest_framework import mixins from rest_framework.viewsets import GenericViewSet

class CargoViewSet(mixins.RetrievModelMixin, mixins.UpdateModelMixin, mixins.CreateModelMixin, GenericViewSet) """Карго вьюха"""

def get_queryset(self, request):
    qs = Cargo.objects.all()
    return qs

def get_serializer_class(self, request):
    match self.action:
        case 'retriev':
            return RetrieveCargoSerializer
        case 'post':
            return CreateCargoSerializer

ну и так далее, можно все в 1 вью уместить.

  1. тут лучше использовать Tupe
  2. fildb.py из названия вообще не понятно что делает этот файл, а внутри половина кода закомментирована, если код закомментирован, его надо удалять
  3. Если пишите docker-compose для Django приложения, то уж наверное нужно его запускать в связке с Nginx, и уж точно не через runserver
  4. Базу данных можно накачать данными локально, потом снять дамп, а в docker-compose поднять postgres уже с дампом, удобнее проверяющему будет
  5. Для применения миграций не нужно делать отдельный контейнер, можно написать  entrypoint.sh на bash и стартовать основное приложение через него
  6. не видно работы с .env переменными (хотя бы django_secret_key туда нужно убрать)
  7. Код не отформатирован, рекомендую использовать black (есть удобные плагины на pycharm или vscode)
  8. тут не должно быть пусто
  9. ридми должно быть более информативно, хотябы с инструкцией как запустить проект. Это все, что сразу бросилось в глаза, на самом деле, будь я нанимающим, закрыл бы через 2 мин, если указываете GH в резюме, то нужно основательно поработать над последними проектами и тогда будет уже что обсудить на собесе. Успехов.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

Lagger0k commented 1 year ago

Как например поднять базу Postgres в докер уже с данными?

Привет, шаги такие, запускаешь проект, как он есть у тебя сейчас, заполняешь Postgres данными, гуглишь команду по снятию с неё дампа и сохраняешь этот дамп в проекте в корне, например db/dump.ddl

а потом в docker-compose монтируешь этот дамп к образу postgres и тогда при старте он будет пробовать создать такую базу внутри с данными, если ее еще нет. Вот пример такого кусочка из компоуса - ./db/init.sql:/docker-entrypoint-initdb.d/init.sql у меня тут init.sql, а у тебя может быть db/dump.ddl

db:
    image: postgres:alpine
    volumes:
      - ./db/init.sql:/docker-entrypoint-initdb.d/init.sql
      - postgres_data:/var/lib/postgresql/data
    env_file:
      - .env