-
Notifications
You must be signed in to change notification settings - Fork 7
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
paper updates #42
paper updates #42
Conversation
# Conflicts: # gefest/core/algs/geom/validation.py # gefest/core/geometry/geometry_2d.py
Hello @quickjkee! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Общие комментарии помимо тех что в коде:
1)В наборе коммитов всё ещё есть лишние (например, мои январские). Возможно часть лишних файлов - из-за них. Предлагаю для удобства схлопнуть все твои правки в 1 коммит.
Алсо, у тебя есть права на этот репозиторий, т.ч. не обязательно из форка работать.
-
Всякие tmp_images - они нужны? Если они служат в качестве примеров - стоит это отразить в названии. Если не нужны - то удалить.
-
Там по мелочи есть предупрежддения по PEP8 - поправь плз.
-
Кое-где у функций и классов потерялись докстринги, type hint-ы и т.п. Если они актуальны - то перенеси их в свою ветку, если нет - то скорректируй или оставь TODO.
@pep8speaks pls update youself |
TODO так TODO, но перед аппрувом поправь плз пункты 1, 2 и 4 (кое-где возвращаемые типы потерялись) из комментария выше. P.S. экшен с гитлабом видимо падает из-за того что у тебя форк. Ну черт с ним. |
No description provided.