Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Edit achievements #437
Edit achievements #437
Changes from 4 commits
4c58a85
1dd4735
be4c7d1
df93d1a
a68db2e
3b4a8fa
883c439
c43a264
8ddf4bb
23abc8c
eb11834
3849707
f9c26ed
fa49f24
a068fe8
439944b
be1689c
023b76d
eeaa715
0b2574e
9568aeb
91289fb
130b868
4b3067c
8c312f8
aa74637
1973456
d1aa574
5b97a44
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Здесь снова используется игнорирование правил. Вообще от WSP в Хекслете мы отказались ,поэтому и в проекте его можно убрать.
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.
пока убирал свои noqa увидел,что во многих местах есть строки закомментированные с A,WPS.
Не убирал в тех местах, которые не касаются моего изменения
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.
ага, тогда можете создать ищщьюс с задачкой почистить от игноров подобные штуки. Затыкать линтер плохая идея, а если он массово затыкается, то может быть это правило не очень нужно (или следует другой линтер использовать)
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.
добавлю новый issue
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.
Добавил
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.
Я бы разделил метод на несколько. Сейчас это огромная стена кода. Выделите подметоды, сделайте их приватными (_method_name), разделите логику.
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.
Вообще тут получилось очень страшно, на будущее его можно переделать.

Списки с дивами не используются. Если у вас список, то между
ul > li
нет блока. Можете посмотреть, как выглядит футер в код бейзиксе или в самом Хекслете.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.
можете завести здесь ищщус на привести футер в порядок
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.
добавлю новый issue
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.
а зачем здесь пустой элемент?
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.
Вот это не поправлено.
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.
элемент списка специально выполнен пустым для того чтобы сохранить оформление
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.
здесь тоже какой-то кастомный styel
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.
перенесено в base.css как кастомный стиль.
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.
здес стиль используется чтобы задать прогерсс бар?
Можно ли расчет width засунуть в хелпер-функцию и здесь просто вызывать?
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.
Да , style="width" отвечает за заполнение полосы прогресса
https://getbootstrap.com/docs/5.0/components/progress/
процедура получения данных для таблицы и сама таблица c личными достижениями сделана по аналогии с achievements.py(было уже разработано раньше), где получали общую статистику.
Непонятно про функцию хелпер: функция должна быть реализована во view и передана в шаблон, где будет получать аргументы для вычисления процента выполнения?
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.
вот насчет джанги не знаю, в других фреймворках (рельсе, ларавел) хелпер обьявляется где-то в модулях приложения и используется во вью.
Если у вас есть другой шаблон, где абсолютно та же логика, но немного другие данные, а вычисления те же - то скорее всего это одна и та же абстракция.
В теории тут можно по всем ачивкам пройтись и сформировать данные для отображения, тогда вы все готовите и передаете в шаблон, который будет слегка чище.
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.
Единственный вариант реализации функции-хелпера, который придумал через @register.simple_tag :
hexlet-friends/contributors/templatetags/contrib_extras.py
Lines 115 to 122 in 883c439
размеры кода остаются те же. объем кода при этом решении почти не сократился.
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.
тут какой-то скастомный стиль используется. Давайте от такого избавляться (он инлайн стилей). Посмотрите, есть ли в бутстрапе похожий класс, если нет, то можно добавить кастомный класс, который именуется через префикс
x-*
, возможно есть другие кастомные классы уже, можно на них посмотреть.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.
А тут не поправили? Посмотрите, как в сикпе сделаны кастомные стили - https://github.com/Hexlet/hexlet-sicp/blob/f02ff453e6c8b045764f497fe536e8ecd24913b6/resources/sass/_custom.scss#L1-L23
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.
style="height: max-content и style="width:50vw перенесены в base.css как кастомные стили.
style="width: {{ contribution|div:achievement_made_count|mul:100|floatformat:0 }} используемый прогрессбаром остается, так как увидел подобное решение в образце от bootstrap