search

alanreidt / range-slider-component

Realization of a range-slider component using Vanilla Javascript. Aimed skills: OOP, MVC, functional programming, Documentation, TDD, UML diagram, Airbnb style guide, etc.
https://alanreidt.github.io/range-slider-component/
MIT License
0 stars 1 forks source link

package.json #9

Closed alagunoff closed 4 years ago

alagunoff commented 4 years ago

Разнеси зависимости на devDep и dep dependencies - зависимости без которых не может обойтись проект devDependencies - зависимости без которых может обойтись проект Пропиши скрипт для линтирования проекта

alanreidt commented 4 years ago

Готово.

alanreidt commented 4 years ago

Коммиты в перемешку вышли, но здесь, если что, изменения по главной ветке (код самого слайдера) и по gh-pages (код для демо страницы).

alagunoff commented 4 years ago

Коммиты в перемешку вышли, но здесь, если что, изменения по главной ветке (код самого слайдера) и по gh-pages (код для демо страницы).

Не надо ничего разделять, делай на 1 изменение 1 коммит. В данном случае хватило бы 2-ух коммитов)

alagunoff commented 4 years ago

У тебя проект может существовать без parcel, sass и т.п., в данном случае? image

alanreidt commented 4 years ago

У тебя проект может существовать без parcel, sass и т.п., в данном случае? image

@madnessJs Да, проект может работать без них в production версии (parcel только собирает проект; sass будет скомпилирован в css), они нужны только при разработке (development).

Ты неправильно понял предназначение этого разделения. Оно просто дает понять npm, какие зависимости требуется загрузить при тех или иных требования. Например, если ты скачал проект только для использования (production версия), то зависимости требуемые при разработке проекта npm скачивать не будет.

alagunoff commented 4 years ago

А какую роль играют эти зависимости в prod сборке? image

alanreidt commented 4 years ago

А какую роль играют эти зависимости в prod сборке? image

@madnessJs Допустим, ты хочешь подключить мой слайдер через npm к своему проекту. Тогда, тебе нужен Lodash, так как он использован в Модели для написания кода в функциональном стиле. @bem-react/classname требуется для составления classNames для разметки и стилей слайдера. deep-equal я планирую использовать для сравнения объектов входящих опций для Модели с текущими и на основе этого производить оповещение о событии update. Но это пока я еще не реализовал.

alagunoff commented 4 years ago

А какую роль играют эти зависимости в prod сборке? image

@madnessJs Допустим, ты хочешь подключить мой слайдер через npm к своему проекту. Тогда, тебе нужен Lodash, так как он использован в Модели для написания кода в функциональном стиле. @bem-react/classname требуется для составления classNames для разметки и стилей слайдера. deep-equal я планирую использовать для сравнения объектов входящих опций для Модели с текущими и на основе этого производить оповещение о событии update. Но это пока я еще не реализовал.

Сейчас понял, спасибо за объяснение. Т.е. когда я подключу твой проект как внешнюю зависимость у меня автоматически скачаются твои зависимости из depenencies?

alanreidt commented 4 years ago

@madnessJs Да. npm документация.

alagunoff commented 4 years ago

Нужно добавить private свойство и убрать main

alagunoff commented 4 years ago

Также убери keywords, version, description

alanreidt commented 4 years ago

@madnessJs Они опциональны. Я заполнил их, чтобы не были пустыми.

alanreidt commented 4 years ago

@madnessJs закрываю?

alagunoff commented 4 years ago

@madnessJs закрываю?

все issues закрываю сам)

alagunoff commented 4 years ago

@madnessJs Они опциональны. Я заполнил их, чтобы не были пустыми.

Какую роль они выполняют в твоем случае?

alanreidt commented 4 years ago

@madnessJs Прямую. Есть в планах опубликовать проект позже.

alagunoff commented 4 years ago

@madnessJs Прямую. Есть в планах опубликовать проект позже.

Убери пока все поля кроме dep, devdep, scripts, private. Как соберешься выкладывать, добавишь

alanreidt commented 4 years ago

Ну а толку мне туда-сюда это все гонять, если оно никакого значения сейчас не имеет?

alagunoff commented 4 years ago

Господи) изучи тогда эту статью, раз тебе нужны на все веские аргументы)

alanreidt commented 4 years ago

Я по ней и ориентируюсь. Единственные необходимые поля — name и version, и то при публикации модуля. Все остальное по-желанию.

alagunoff commented 4 years ago

Я по ней и ориентируюсь. Единственные необходимые поля — name и version, и то при публикации модуля. Все остальное по-желанию.

зачем ты тогда эти исправления внес?) image

alanreidt commented 4 years ago

Потому что main в рамках данного проекта совсем не нужен. А private полезен будет. Не пойму, что ты хочешь этим доказать.

alagunoff commented 4 years ago

Потому что main в рамках данного проекта совсем не нужен. А private полезен будет. Не пойму, что ты хочешь этим доказать.

'main совсем не нужен'. А description, к примеру, нужен но не совсем?) main то ты убрал, а за другие поля горой стоишь, почему?)) Я не доказываю ничего, просто пытаюсь понять тебя по этому вопросу

alanreidt commented 4 years ago

@artamonJr Main не подходит по своему предназначению. В документации оно немного расплывчато описано, но, в кратце, мне в проекте нужно поле browser использовать, так как есть зависимость от окружения браузера. То есть, main свойство, при любом раскладе лишнее. А другие поля понадобятся, когда нужно будет опубликовать проект.

alanreidt commented 4 years ago

Но я вижу теперь в чем проблема. Я так понимаю, что проверяющим толком не объясняли в чем их задачи на ревью?

Видишь ли, в рамках данной правки, ключевой задачей является проверить мое понимание базовых требований для package.json при работе с проектами (хотя я не уверен, что данные требования установлены внутри команды). А именно: scripts, поля связанные с окружением проекта (линтеры, тесты...), знание о существовании поля private (рабочие проекты в npm не публикуются), правильное разбиение зависимостей (важно даже в приватных проектах, так как позволяет установить только production зависимости при проверке проекта (при желании)). Остальное не важно, если не стоит задачи публикации (в моем случае она присутствует).

alanreidt commented 4 years ago

И, по этой же причине, по остальным issues много чего лишнего происходит. Но это уже вопрос к Сергею.

alagunoff commented 4 years ago

Антон ты сам задал этот стиль общения, а сейчас, когда с тобой общаются в стиле твоего общения - ты не доволен) Ты пытаешься быть настолько умным, насколько ты сам себя видишь, но это может в некоторых случаях сыграть злую шутку(в этом). Ты даже знаешь, что входит в наши обязанности как ревьюиров) Может напишешь Сергею сразу, чтобы тебя так взяли в команду, без проверки? Тк ты все и так сам знаешь)

alagunoff commented 4 years ago

Нужно просто быть немного проще, а не бегать к кому-то за помощью, чтобы тебе помогли уладить вопросы

alanreidt commented 4 years ago

@artamonJr Давай без оскорблений. Я ни к кому за помощью не бегал, а уточнял возникший вопрос. Мне не понятно было откуда ты взял требования добавления доп. функциональности. Если в случае с зависимостями был абсолют — документация, то в этом случае мог помочь только Сергей.

Я, по-правде, не вижу какой-либо направленности в твоих требованиях. Удаление, выше упомянутых, полей ничего совершенно не изменит (кроме того, что мне потребуется позже добавить их обратно). Поэтому, я и заключил, что задачи у вас не поставлены, либо очень свободны.

А требования по данной правке (и в общем) легко установить логически, исходя из целей заданий данной программы.

alagunoff commented 4 years ago

@alanreidt Хорошо, закрыли эти принципиальные моменты. Задачи у нас можно сказать свободны в целом, да. Руководствуемся только своим опытом и best practices компании. Согласен с тобой, возможно я пытался подвести тебя под свои рамки и не учитывал, что люди по-разному смотрят на вещи) Будет круто, если будешь отписываться в телеграмме, как можно проверять