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

Task 1 #1

Merged
merged 12 commits into from
Mar 1, 2024
Merged

Task 1 #1

merged 12 commits into from
Mar 1, 2024

Conversation

TreshMom
Copy link
Owner

@TreshMom TreshMom commented Mar 1, 2024

No description provided.

@TreshMom
Copy link
Owner Author

TreshMom commented Mar 1, 2024

Здравствуйте!
Для запуска тестов :
1)export PYTHONPATH='PATH/TO/PROJECT' (саму папку project не надо)
2)pytest tests/task1_test.py
Рабочую версию я запушил до дедлайна, насколько помню, потом просто поправлял.

@TreshMom TreshMom requested a review from WoWaster March 1, 2024 10:52
Copy link

@WoWaster WoWaster left a comment

Choose a reason for hiding this comment

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

Кроме того, везде отсутствует перенос строки в конце файла 😢

Copy link

Choose a reason for hiding this comment

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

Ай-яй. Некрасиво выключать проверку стиля кода

path=path,
)

create_labeled_two_cycle_graph(3,4,("a","b"),'1')
Copy link

Choose a reason for hiding this comment

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

Довольно странное решение вызывать библиотечную функцию на топлевеле в самой библиотеке

@TreshMom
Copy link
Owner Author

TreshMom commented Mar 1, 2024

Не специально отключил проверку стиля, правильно понимаю, мне надо включить проверку стиля, отредактировать code style, поправить тест и после принятия task 1, залить task 2 (его приняли?)?

@WoWaster
Copy link

WoWaster commented Mar 1, 2024

Вам нужно

  • вернуть проверку стиля
  • поправить стиль кода, если на вас наругается pre-commit
  • убрать или не убирать вот ту строчку, на которую я уже указал
  • попросить новый ревью от меня

Если тут будет всё хорошо, и я дам добро, то вам нужно будет влить эту ветку в main, а потом подтянуть main в ветку с задачей 2, чтобы я не видел лишнего кода в изменениях

@TreshMom
Copy link
Owner Author

TreshMom commented Mar 1, 2024

Исправил все недочеты, запустил black на всем проекте, но pre-commit все равно на что-то ругается...
Вы сказали, после влить 2 ветку, в 1, разве получиться замержить main.py? @WoWaster
Снимок экрана 2024-03-01 в 20 53 03
Снимок экрана 2024-03-01 в 20 53 08

@WoWaster
Copy link

WoWaster commented Mar 1, 2024

Логи pre-commit подсказывают чего делать. Вам стоит запускать именно pre-commit, а не только black. Первый проверяет и остальные файлы, помимо питоновских.

Ваш план действий сейчас такой:

  1. Я (в будущем) одобряю данный PR
  2. Вы его вливаете в свой main
  3. Подтягивает main во вторую ветку

Merge conflict может возникнуть, но навряд ли там будет что-то ужасно сложное для починки

Copy link

@WoWaster WoWaster left a comment

Choose a reason for hiding this comment

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

Поздравляю! Вы смогли!

@TreshMom
Copy link
Owner Author

TreshMom commented Mar 1, 2024

спасибо за подсказки! в следующих домашках будет лучше.
Уточняю, вливаю task-1 в main, потом task-2 также в main?
Просто вы еще написали в тг, что можно вливать task-2 в task-1, по факту разницы никакой, но уточнить надо

@WoWaster
Copy link

WoWaster commented Mar 1, 2024

Да, обе домашки давайте сейчас вливать в main, так будет проще

@TreshMom TreshMom merged commit d99ddfa into main Mar 1, 2024
4 checks passed
@TreshMom TreshMom deleted the task-1 branch March 1, 2024 18:41
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.

2 participants