Skip to content

Conversation

binjospookie
Copy link
Contributor

close #71

@azinit
Copy link
Member

azinit commented Mar 21, 2021

@binjospookie пингани (или кинь в чат), как будет готово к ревью

@binjospookie
Copy link
Contributor Author

binjospookie commented Mar 21, 2021

@martis-git давай считать за преревью)

у меня есть пара вопросов:

  • надо ли раскрыть ещё какие-то аспекты?
  • раскрыты ли достаточно описанные аспекты?

@akaia-shadowfox akaia-shadowfox added this to the 2.0.0-alpha.1 milestone Mar 21, 2021
@azinit azinit requested review from a team, AlexandrHoroshih and akaia-shadowfox and removed request for a team March 22, 2021 00:30
@azinit
Copy link
Member

azinit commented Mar 22, 2021

Copy link
Member

@azinit azinit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@binjospookie в целом то неплохо, но много что доделать бы еще)

  1. В целом, повторюсь, советую ознакомиться с тем, как это мы с ребятами делали уже (тут прям оч хорошо прослеживаются те правки которые я накидал тебе)

  2. Как минимум то что сразу в глаза бросается - доработать бы в плане структуры и readability для пользователя

    Сейчас же хедеров нет и структура не прослеживается
    А "подробная инфа", к-рая прям всем-всем пользователям не понадобится - не скрывается в спойлерах

  3. Вот все 11 правок - это только по форме было)
    Если же смотреть по тому "чего не хватает содержательно" - то будет еще 20, но для этого как минимум придется еще за тебя работу проделать

    Да и ты задолбаешься 30 чужих правок вносить, нежели сам написать сразу полноценно))

    Поэтому прошу еще раз пройтись по изначальной дискуссии и посмотреть - какие тезисы там выдвигаются

    Пока же, извини, возникает впечатление, что ты из своих ощущений писал больше

UPD: Можно пройтись по ~готовой доке, где тоже "бизнес-ценности" и "функциональность" разбираются

https://github.com/feature-sliced/wiki/blob/docs/concepts/understanding-needs/concepts/understanding-needs.md

image

@@ -0,0 +1,50 @@
Feature — изменение в продукте, которое несет ценность пользователю.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вынес бы в concepts/abstractions/feature.md

@@ -0,0 +1,50 @@
Feature — изменение в продукте, которое несет ценность пользователю.
Copy link
Member

@azinit azinit Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не хватает хедера с overview секцией

Suggested change
Feature — изменение в продукте, которое несет ценность пользователю.
# Feature
## Описание
`feature` — изменение в продукте, которое несет ценность пользователю.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь бы еще описания добавить, глянь как у @karina-drummer сделано)

image

Comment on lines +3 to +7
Рассмотрим некоторые GitHub features:

- issues
- actions
- marketplace
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я бы вынес такое отдельным пунктом в ## Примеры

И тогда можно не только про GitHub сказать, но и про YouTube, и да - тот же маркетплейс

В принципе, если этот раздел будет до основного текста - вроде нормально по читаемости и смыслу будет

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тип как "Примеры из жизни"

Comment on lines +9 to +27
Features могут перекликаться с другими features. Например, marketplace позволяет найти готовый action, а не писать свой.

Feature может содержать другие features: при создании issue можно проставить labels, assignees, milestone.

Таким образом приходим к следующей структуре:

```
.
├── issues/
│ ├── assignees
│ ├── labels
│ └── milestone
├── actions/
├── marketplace/
│ ├── list
│ ├── search
│ └── filter
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Да, понимаю что у тебя была подводка на основании примеров, но надо бы как-то более структурированно разделить и описать что есть фича (более содержательно)

См. как @AlexandrHoroshih сделал

  1. Про структуру то что ты описал - это хорошо, но именно данная структура больше относится к nested-features

Т.е. я бы добавил ее как раскрывающийся спойлер

  1. Про nested-features - по-моему была речь на одном из созвонов, что nested-features у нас как таковых нет, у нас максимум - фичи могут "группироваться" по какой-то доменной зоне (например, условно feature/auth/by-phone, feature/auth/by-oauth и т.д.)

При этом группировка - это значит что просто фичи кладутся в одну папку, НО НЕ БОЛЕЕ

Иначе будет большое желание связывать эти фичи тем или иным способом - а вот отсюда и возникает большая часть проблем

Copy link
Contributor

@ilyaagarkov ilyaagarkov Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

├── issues/
│   ├── assignees
│   ├── labels
│   └── milestone
├── actions/
├── marketplace/

это же все как раз не фичи, а сущности над которыми и строятся фичи.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вай нот? Возможность создать issue -- фича. Возможность повесить label на issue -- тоже фича.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

создать issue это фича, но issue сами по себе это не фича.
Подобные фичи ведут как раз к необходимости импортить фичи из фич. И как раз эта проблема и привела к появлению такой сущности как entities

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Готовые issue — результат создания.

Возможность создания — фича.
Возможность повесть label — фича.

Тот же label несёт пользователю ценность, например, можно указать тип issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот именно что не сам label несет ценность, а именно возможность повесть лейбл на что-то несет ценность.
В общем я свое мнение по поводу этого писал тут #23 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лейбл в контексте issues.

Будь ещё какой-то лейбл, то, да, его частя болталась бы в entities.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут больше с @ilyaagarkov соглашусь

Да и в целом у нас больше людей в core-team, которые считают что фича - конкретная бизнес-ценность и действие (здесь кста тож про это писал)

Я с entities плотно не работал, тож обычно похожим образом на фичи разбивал (labels-list, issue-details и т.д.)

Но похоже на правду - что при разбиении как предлагает @ilyaagarkov и другие ребята (entities + features) - архитектура будет более масштабируемой, читаемой и при этом - сведутся к минимумы необходимости кросс-импортов (а это главный бич сейчас у похожих подходов)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лейбл в контексте issues.

Будь ещё какой-то лейбл, то, да, его частя болталась бы в entities.

Ну если у тебя лейбл только для ишью (хотя обычно тот же лейбл и на пуллреквесты вешается), то можно его issue-label назвать

Чтобы более плоская структура получалась, и не нужно было вкладывать

Хотя с другой стороны, это уже похоже на проблему nested-entities

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@binjospookie @ilyaagarkov можем обсудить попозже в войсе

│ └── filter
```

Как выглядит feature в реализации?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А вот здесь как раз можно выделить секцию для структуры

## Реализация

...


Как выглядит feature в реализации?

Зачастую, это логическая часть и способ взаимодействия с ней.<br />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

По-моему хватит просто переноса на след. строку ,без br

Comment on lines +31 to +42
Логическая часть = feature `model`. Это место, где описаны правила, по которым существует feature.

Рассмотрим на примере формы аутентификации:

- хранилище значений полей формы (email, password)
- хранилище валидности формы (ошибки в полях)
- события обновления хранилищ
- эффект отправки формы

Этому слою feature не важно как с ним взаимодействует пользователь. Через браузер или через командную строку.

Посредником между пользователем и feature model выступает визульный слой: `UI`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот я бы прям на верхнем уровне описал что-то вроде

По реализации `feature` может иметь любые низкоуровневые абстракции (ui, lib, api, model), но чаще всего ограничивается двумя:
- `ui/` - логика отображения фичи
   > Добавляем спойлер с более подробной инфой, который не должен глаза мазолить
- `model/` - бизнес-логика фичи
   > И здесь снова спойлер - сюда бы как раз добавил пункт "Рассмотрим на примере формы аутентификации: ..."

├── feature/
│ ├── model (logic)
│ └── ui (visual)
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Посмотри как в README сделано примерно

  • ```sh
  • ├── model # Business logic

├── feature/
│ ├── model (logic)
│ └── ui (visual)
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не хватает в конце раздела что-то вроде Further Reading (опять же глянь примеры которые кидал выше)

Как минимум добавить ссылка на изначальную дискуссию должна быть

## 📑 См. также
<!-- TODO: Возможно позже надо вынести в md-var -->
- [*Обсуждение* "Что есть 'feature'?"](https://github.com/feature-sliced/wiki/discussions/23)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@azinit azinit added the help wanted Extra attention is needed label Mar 25, 2021
@azinit azinit removed the help wanted Extra attention is needed label Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Добавить доку: "Что есть feature"

4 participants