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

Full review of repository code by Denys Velychko #1

Open
wants to merge 136 commits into
base: Initial-commit
Choose a base branch
from

Conversation

Shushpin
Copy link
Owner

@Shushpin Shushpin commented May 25, 2023

Взагалі красава, проект вартує 51-го балу +. Я б такого не зробив, як мінімум через те, що не вчу пітон. Ти молодець, продовжй працювати над проектом.

Copy link
Owner Author

@Shushpin Shushpin left a comment

Choose a reason for hiding this comment

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

Взагалі красава, проект вартує 51-го балу +. Я б такого не зробив, як мінімум через те, що не вчу пітон. Ти молодець, продовжй працювати над проектом.


class Post(db.Model):
id = db.Column(db.Integer, primary_key=True)
title = db.Column(LONGTEXT, nullable=False)
Copy link
Owner Author

Choose a reason for hiding this comment

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

В класі User ти використовуєте db.String(300) для поля password, що вказує максимальну довжину рядка у 300 символів, зазвичай, для збереження хешів паролів використовуються поля з достатньою довжиною, щоб помістити хеші будь-якої довжини.Я б рекомендується використовувати більшу довжину, наприклад, db.String(512) або використати тип даних, який безпечніше зберігати хеші паролів, такий як db.Binary або db.LargeBinary.

from app import create_app,db
from flask_migrate import upgrade,migrate,init,stamp
from models import User

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ти використовуєте app.app_context().push() для створення контексту додатка перед створенням таблиць бази даних. Це може бути зайвим, оскільки app.app_context() автоматично викликає push() для контексту додатка. Можна просто викликати app.app_context() без виклику push().

db.create_all()

# migrate database to latest revision
init()
Copy link
Owner Author

Choose a reason for hiding this comment

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

Потрібно виконати міграцію бази даних перед оновленням (upgrade()) для переконанняся, що структура бази даних відповідає останній ревізії. Можна викликати migrate() перед upgrade().

@@ -0,0 +1,19 @@

def deploy():
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ти повторно імпортуєте модуль app на початку функції deploy(). Це може призвести до проблем з виконанням коду, якщо модуль app вже імпортований в головному файлі програми. Рекомендується імпортувати модуль app поза функцією deploy(), наприклад, в головному файлі програми або в файлі, який викликає функцію deploy().

forms.py Outdated
Copy link
Owner Author

Choose a reason for hiding this comment

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

Код виглядає красиво і зрозуміло, але ти використовуєш повторювані значення, такі як мінімальна і максимальна довжина рядка для валідаторів Length. Для полегшення зміни цих значень у майбутньому рекомендується використовувати константи або змінні для їх зберігання.

forms.py Outdated
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ще проблема у використанні іменованих валідаторів. У тебе є декілька валідаторів з однаковими параметрами для різних полів форми. Це можна спростити, використовуючи іменовані валідатори за допомогою wtforms.validators модуля.


load_dotenv() # зчитує значення з файлу .env

class Config:
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ти можете використати параметри за замовчуванням для os.getenv(), щоб задати значення за замовчуванням, якщо змінна середовища не знайдена. Це забезпечить більшу гнучкість, коли ти працюєш з різними середовищами

@@ -0,0 +1,19 @@
{% extends 'base.html' %}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Взагалі прикольно зроблено, але можна ще використати форматування рядків для класів CSS та використати блоки для кращої організації коду, типу такого:
{% block form_fields %}

{{ form.title.label(class_="form-label") }}
{{ form.title(class_="form-control", value=post.title) }}

{{ form.content.label(class_="form-label") }}
{{ form.content(class_="form-control", rows=5)}}

{% endblock %}

Copy link
Owner Author

@Shushpin Shushpin left a comment

Choose a reason for hiding this comment

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

Взагалі красава, проект вартує 51-го балу +. Я б такого не зробив, як мінімум через те, що не вчу пітон. Ти молодець, продовжй працювати над проектом.

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