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

Mlynarchik Artyom #40

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

Archeex
Copy link

@Archeex Archeex commented Nov 10, 2019

For install: pip install . -r requirements.txt
Small imperfection: file with font should be locate in directory from which launch module.
Codestyle checked with pycodestyle.
Module date_validation is redundant, i know :)

Unfortunately I didnt have time to fix saving 'JPG' images to PDF-file (only 'PNG' working, because i choose bad module).
5/6 iterations completed.

@AlexeiBuzuma AlexeiBuzuma added the [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. label Nov 17, 2019
setup.py Show resolved Hide resolved
@Archeex Archeex changed the title [WIP] Mlynarchik Artyom Mlynarchik Artyom Dec 1, 2019
@AlexeiBuzuma AlexeiBuzuma removed Hard Deadline [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. labels Dec 12, 2019
@AlexeiBuzuma
Copy link
Collaborator

Моменты, которые можно улучшить:

  1. HTML собирается "руками". На сегодняшний день есть большое количество уже готовых библиотек, которые помогут сделать это удобнее, красивее и безопаснее.
  2. Не совсем правильное использование метода "str": https://docs.python.org/3/reference/datamodel.html#object.__str__
  3. слишком много вызовов функций exit(). В приложении такого размера можно обойтись одной точкой выхода из программы.
  4. Тестов могло быть больше :)
  5. Зачастую функцию названы одним словом, например format и cahce. Я бы советовал давать более подробные имена аттрибутам.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants