Skip to content

Добавляет новые правила под новые критерии #52

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
Merged
merged 32 commits into from
Jun 23, 2021
Merged

Добавляет новые правила под новые критерии #52

merged 32 commits into from
Jun 23, 2021

Conversation

nikolai-shabalin
Copy link
Contributor

No description provided.

@nikolai-shabalin nikolai-shabalin marked this pull request as ready for review February 17, 2021 15:23
tenthreee
tenthreee reviewed Feb 17, 2021
View reviewed changes
nikolai-shabalin and others added 8 commits February 17, 2021 18:54
@nikolai-shabalin
Update html-css.html
18f994c 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
e367dce 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
3539083 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
1188224 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
1103d22 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
89cf384 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
7a70844 
Co-authored-by: Lena Morozova <[email protected]>
@nikolai-shabalin
Update html-css.html
adfc476 
Co-authored-by: Lena Morozova <[email protected]>
@polyuretanio
Copy link
Member

polyuretanio commented Apr 1, 2021 •
edited by nikolai-shabalin
Loading

  • От формулировок "Правильно/неправильно" лучше вернуться к формулировкам "Хорошо/плохо"
  • В автоматизации проверок кода надо сказать, что это не ссылки на сами инструменты, а ссылки на конфигурации аккадемии для этих инструментов. И эти конфигурации академии частично покрывают то, что описано в кодгаде ниже. И в конфигурациях нет ничего, что не описано ниже.
  • Кодгайд более рекомендательная штука, поэтому можно использовать более мягкую модальность. То есть уйти от "должны". Например, вместо "Файлы схожих типов должны быть сгруппированы в папки: картинки, стили, скрипты, шрифты." можно написать "Файлы схожих типов группируются в папки: картинки, стили, скрипты, шрифты."
  • странный пункт про "Форматирование кода" и эдитор конфиг. Кодгайд про конкретику, а в этом пункте нет никакой конкретики
  • Форматирование тегов здесь у меня первое расхождение с кодгайдом нашим, я не всегда переношу каждый тег на новую строку и считаю, что это вполне ок
  • Есть ощущение, что многие отдельные пункты новой версии можно сгруппировать в списки для большей компактности, как в старом кодгайде
  • "Единицы измерения в атрибутах" а если есть такие атрибуты, в которых единицы измерения это ок? Да и вообще это разве пункт кодгайда? Это же просто спекой запрещено
  • "Подписи полей ввода" это дубль критерия, зачем он в кодгайде?
  • "Подключение шрифтов" тоже дубль критерия и непонятно зачем это в кодгайде
  • "Варианты шрифта" надо обновить в сооветствии с критерием, и это тоже дубль

Резюмируя: 99% вышеперечисленного это комментарии по форме представления кодгайда, а не по его содержанию

@nikolai-shabalin
Copy link
Contributor Author

@polyuretanio

странный пункт про "Форматирование кода" и эдитор конфиг. Кодгайд про конкретику, а в этом пункте нет никакой конкретики

А чего не хватает? Вроде всё явно. Следуй эдиторкофигу. Прямое требование

@polyuretanio
Copy link
Member

@polyuretanio

странный пункт про "Форматирование кода" и эдитор конфиг. Кодгайд про конкретику, а в этом пункте нет никакой конкретики

А чего не хватает? Вроде всё явно. Следуй эдиторкофигу. Прямое требование

Просто это не выглядит куском кодгайда. Должен быть пример конкретного конфига. А не просто "используйте такой-то инструмен". Точно также можно попросить "использовать препроцесор" или "использовать методологию названия классов", причем без уточнения, что за препроцессор или методология.

@nikolai-shabalin
Copy link
Contributor Author

Угу, понял

@nikolai-shabalin nikolai-shabalin merged commit 5fe622c into master Jun 23, 2021
@nikolai-shabalin nikolai-shabalin deleted the feature/new-rule branch June 23, 2021 07:28
nikolai-shabalin added a commit that referenced this pull request Jun 23, 2021
@nikolai-shabalin
Добавляет новые правила под новые критерии (#52)
28cdac8 
* Добавляет новые правила под новые критерии

* Заменяет Барбершоп на project

* Уточняет про camelCase атрибуты

* Разбивает CSS-синтаксис на несколько пунктов

* Удаляет дубликаты у HTML

* Удаляет дубликаты у CSS

* Заменяет хорошо/плохо на правильно/неправильно

* Форматирует HTML-правила

* Форматирует CSS-правила

* Добавляет про минимальную структуру HTML-документа

* Добавляет линтеры

* Меняет правильно/неправильно на хорошо/плохо

* Заменяет должность на более мягную модальность

* Добавляет уточнение про конфигурации

* Убирает единицы измерения в атрибутах

* Уточняет форматирование тегов

* Конкретизирует пункт про форматирование кода

* Обновляет порядок для текстовый свойств

* Улучшает пример "Регистр тегов и атрибутов"

* Группирует правила для HTML

* Группирует правила для CSS

* Исправляет пример для регистра

* Обновляет пример форматирования кода

(cherry picked from commit 5fe622c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tenthreee tenthreee tenthreee left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nikolai-shabalin @polyuretanio @tenthreee