-
Notifications
You must be signed in to change notification settings - Fork 8
Стайлгайд для докблоков #24
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,77 @@ | |||
# Почему докблоки не нужны |
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.
Возможно, текст лишний, это скорее для того чтобы не переспрашивали 10 раз
- `@Route` в контроллерах | ||
- `@Entity`, `@Embeddable`, `???` в сущностях | ||
- `@Constraint` | ||
- ??? |
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.
Тут не уверен, что всё нужное перечислил
|
||
## `@inheritDoc` | ||
|
||
Не нужен (кроме код-ревью???), зато может уронить билд за несоблюдение кодстайла, если написать без фигурных скобок. |
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.
Кто-нибудь на код-ревью ощущал профит от того, что прописаны @inheritdoc
?
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.
Если PHPStorm подсвечивает без него, тогда не нужен.
class SomeClass | ||
``` | ||
|
||
Каковые конструкции: |
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.
Может горизонтальный стиль вообще убрать? Симфони, например, считает не кошерным использование инлайн PHPDoc'ов
Don't inline PHPDoc blocks, even when they contain just one tag (e.g. don't put
/** {@inheritdoc} */
in a single line);
Понимаю, что от каких то правил связанных с PHPDoc можно отказаться. Но в данном случае, думаю имеет смысл оставить один возможный вариант, чтобы уж наверняка избежать разного кодстайла в проекте.
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.
+1 за вертикальный, только.
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.
Блин, писать 3 строки комментариев для свойства класса, которое занимает одну строку - такое.
Однако: | ||
- сам документ, как и PSRы, составлялся во времена PHP < 7, когда не было другого способа полностью документировать возвращаемые типы и аргументы; | ||
- обширная документация имеет куда большую важность для многократно используемой библиотеки (фреймворки, бандлы) | ||
- в архитектуру, в интересах BC, нечасто вносятся серъезные изменения, комментарии имеют меньше шансов устареть по недосмотру |
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.
Почему эта ненужная? Мне казалось, она достаточно прижилась.
- зато очень важно указать в каждом классе лицензию, версию, автора, его email ~и кличку любимой собаки~ | ||
|
||
Тогда как: | ||
- в частных, не-коробочных проектах код носит «штучный» характер, который предполагает куда меньше вариантов (ре)использования; |
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.
Should.
|
||
Документировать, когда: | ||
- информацию о его предназначении не выразить в имени класса ± неймспейса, или нет на это времени; | ||
- нужно указать примеры использования, а времени писать тесты — нет; |
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.
Should.
- `@Constraint` | ||
- ??? | ||
|
||
Есть еще «волшебные» `@method` и `@property`, но за их применение предлагается сажать на бутылку. |
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.
Убрать, т.к. документ и так запрещает все, что не разрешено. Нет необходимости запрещать что-то повторно.
Документировать метод нужно, если: | ||
- хотя бы один из параметров может быть массивом / итератором элементов строго определенного типа (`string[]`, ` Collection|SomeClass[]`) | ||
- возвращаемый тип может быть таким же массивом/итератором | ||
- нужно указать тип выбрасываемого исключения (зачем???) |
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.
TBD. Вопрос: входит ли указание исключений в пхпДоках в сам интерфейс? Нужно ли их там указывать?
|
||
## `@inheritDoc` | ||
|
||
Не нужен (кроме код-ревью???), зато может уронить билд за несоблюдение кодстайла, если написать без фигурных скобок. |
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.
Если PHPStorm подсвечивает без него, тогда не нужен.
|
||
## Переменные | ||
|
||
Следует максимально избегать, где позволяют возможности IDE / документирования иными способами. |
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.
Нужно конкретизировать.
|
||
Заодно меньше проблем с `private: true` по умолчанию и рефакторинге именованных сервисов. | ||
|
||
##### Репозитории, получаемые `$this->getDoctrine()->getRepository($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.
Спорный вопрос. Решили проверить на практике, насколько готовы сейчас от этого отказаться и насколько неудобно будет без него.
|
||
См. предыдущий пункт. | ||
|
||
##### Обход циклом данных из итератора |
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.
Можем отказаться от этого, если решим вопрос по инлайн-докам для переменных из репозитория и т.п.
Как там с PR? |
No description provided.