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.

Sign up for GitHub

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

Jump to bottom

ВСК-35 Добавление pre-commit #84

Merged
merged 8 commits into from
Jun 3, 2024
Merged

ВСК-35 Добавление pre-commit #84

merged 8 commits into from
Jun 3, 2024

Conversation

NECROshizo
Copy link
Collaborator

  • Добавлены пре комит хуки
    • check-yaml проверка yaml
    • end-of-file-fixer добавляет пустую строку в конце при необходимости
    • trailing-whitespace удаляет пробелы в конце строк
    • check-merge-conflict проверка на метки слияния
    • double-quote-string-fixer " ===> '
  • Автоформотирования импортов isort
  • flacke8 общая проверка + плагины
    • pep8-naming проверка имен переменных
    • flake8-isort доп проверка порядка импортов на всякий случай
    • flake8-broken-line проверка правильности переноса строк
    • flake8-bugbear потенциальные доработки в коде
  • добавлен pytest к хукам
  • пара ручных правок.
  • перенос всех конфигов в setup.cfg
  • в readme справка по использованию pre-commit

P.S. файлы интереса .pre-commit-config.yaml, setup.cfg, ручная правка на сырую строку app/services/mail.py

@NECROshizo NECROshizo added the enhancement New feature or request label May 19, 2024
@NECROshizo NECROshizo changed the title Feature/bck 36 pre commit ВСК-35 Добавление pre-commit May 19, 2024
MikhailDevW
MikhailDevW requested changes May 20, 2024
View reviewed changes
Copy link
Collaborator

@MikhailDevW MikhailDevW left a comment

Choose a reason for hiding this comment

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

Годно. В проекте появляется порядок

README.md Outdated

- Установить pre-commit в проекте: `pre-commit install`
- Запустить проверку всех хуков: `pre-commit run -a`
- Запустить конкретный хук: `poetry run pre-commit run <имя-хука>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Я когда пришел в проект, как я понял все сразу запускалось и работало только в докере.
И там во флоу используется пипка. Тут поетри. С чем это связано и надо ли?
Дополнительно я думал а почему бы не сделать скрипт запуска проекта когда на машине он запускается и если есть питончик тос тавит виртуальное окружение, все зависимости и настраивает проект, включая пре-коммит, чтобы руками каждый раз этого не делать. На обсуждение.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Нет, тут я свою памятку копировал и не углядел.

app/admin/authentication.py Outdated
@@ -16,14 +16,14 @@ async def login(
strategy: JWTStrategy = get_jwt_strategy()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Этот файл трогался только для рефача однако тут отступы не 4 пробела.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Эти строки мной не менялись. Так было самого начала, вообще странные артефакты постоянно встречаются.

app/admin/authentication.py
@@ -37,7 +37,7 @@ async def authenticate(
request: Request,
session: AsyncSession = AsyncSessionLocal()
) -> bool:
token = request.session.get("token")
token = request.session.get('token')
if not token:
return False
return True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут и формы и токен. Странный файл....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да

app/api/endpoints/register.py
@@ -48,8 +48,8 @@ async def register(
raise HTTPException(
Copy link
Collaborator

Choose a reason for hiding this comment

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

в этом файле тоже не 4 пробела отступы да и везде по проекту

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, но в этой строке не вижу проблем.

tests/test_group.py Outdated
@@ -46,7 +46,7 @@ async def test_create_group(
db_session: AsyncSession,
auth_superuser: TestClient
):
"""Создание группы суперюзером"""
"""Создание группы суперюзером/."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

слеш это опечатка?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Да, поправил.

setup.cfg
max-complexity = 9
max-line-length = 79

[tool:pytest]
Copy link
Collaborator

Choose a reason for hiding this comment

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

А вот тут для моего понимания обязательно писать tools.?? флейк и исорт без этого указания. Не будет так работать?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pytest согласно документации, все остальные разрешают так.
setup.cfg потому что pyproject.toml не поддерживается доживающим старичком flake8.

D-Abramoc
D-Abramoc approved these changes May 22, 2024
View reviewed changes
Copy link
Collaborator

@D-Abramoc D-Abramoc left a comment

Choose a reason for hiding this comment

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

Я пока не совсем понял для чего это. Видимо это можно запустить перед коммитом и всякие мелкие штуки сами поправятся. Наверное норм

@NECROshizo
Copy link
Collaborator Author

NECROshizo commented May 26, 2024 •
edited
Loading

Я пока не совсем понял для чего это. Видимо это можно запустить перед коммитом и всякие мелкие штуки сами поправятся. Наверное норм

Предполагается что инициализируются хуки pre-commit install, и после этого любой комит не будет исполнен пока не пройдет все тестирование как кодстайлом так и тестами. + авторформатеры вносят изменения. Это обычно делается в начале проекта, чтобы код был в едином стиле в дальнейшем.

@NECROshizo NECROshizo requested a review from MikhailDevW May 26, 2024 18:09
@kokhlo kokhlo self-assigned this May 29, 2024
D-Abramoc added 3 commits May 29, 2024 17:42
@D-Abramoc
Merge branch 'develop' into feature/BCK-36_pre_commit
0a151bb
@D-Abramoc
Update requirements.txt
28f302e
@D-Abramoc
Update test_achievement.py
ceb0a72 
В тесте пагинации при запросе добавил await
@kokhlo kokhlo merged commit 368d5e0 into develop Jun 3, 2024
3 checks passed
@kokhlo kokhlo deleted the feature/BCK-36_pre_commit branch June 3, 2024 15:55
AndreyHlim pushed a commit that referenced this pull request Jun 27, 2024
@kokhlo @AndreyHlim
Merge pull request #84 from QActf/feature/BCK-36_pre_commit
025f703 
ВСК-35 Добавление pre-commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@D-Abramoc D-Abramoc D-Abramoc approved these changes

@MikhailDevW MikhailDevW MikhailDevW approved these changes

@kokhlo kokhlo Awaiting requested review from kokhlo

@Creee9 Creee9 Awaiting requested review from Creee9

Assignees

@kokhlo kokhlo

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NECROshizo @MikhailDevW @D-Abramoc @kokhlo