fullstack-development / react-redux-starter-kit

Modular starter kit for React+Redux+React Router projects.
https://demo.fullstack-development.com/
MIT License
91 stars 13 forks source link

104 add prettier #111

Closed chmnkh closed 4 years ago

chmnkh commented 5 years ago

На пре-коммит добавил:

На пре-пуш добавил:

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

Коммон конфиг через преттиер прогнал, больше ничего не стал, ибо там под сотку изменений тогда)

alextsk commented 5 years ago

ибо там под сотку изменений тогда

так это нормально, их нужно сделать один раз для всего проекта

chmnkh commented 5 years ago

так это нормально, их нужно сделать один раз для всего проекта

будет тупо неудобно ревьюить тогда то, что я добавил в рамках таски если прямо будут все топить что надо перевести проект, то я одним пр-ом могу тупо под преттиер подогнать, но не уверен, насколько это надо

clicktronix commented 5 years ago

А расширение для вскода не тестил? Он раньше вроде тоже юзал .prettierrc

chmnkh commented 5 years ago

А расширение для вскода не тестил? Он раньше вроде тоже юзал .prettierrc

у меня он есть, но я им тупо цсс-правила местами переставляю, когда стайллинт ругается на их порядок, для остального я его оффнул вообще

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

Znack commented 5 years ago

Лучше отдельным pr все файлы отформатить, да

чт, 11 апр. 2019 г. в 15:50, Alexey Cheremnykh notifications@github.com:

А расширение для вскода не тестил? Он раньше вроде тоже юзал .prettierrc

у меня он есть, но я им тупо цсс-правила местами переставляю, когда стайллинт ругается на их порядок, для остального я его оффнул вообще

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fullstack-development/react-redux-starter-kit/pull/111#issuecomment-482027460, or mute the thread https://github.com/notifications/unsubscribe-auth/ACyxREtgH3vaOaKn5G4efDQsVGNsrVeMks5vfvdugaJpZM4co5Zk .

in19farkt commented 5 years ago

Не согласен с тем что нужно отдельным ПРом форматить. Лучше сделать отдельный коммит в рамках этого ПРа. А то этот ПР не понятно как принимать, потому что мы не видим притир в деле.

Znack commented 5 years ago

Тут будет тяжело отличать реальные изменения в конфигах от фотмартирования. Просто коммит не пойдет, так как сейчас вылезут нюансы и это будет уже не один коммит, а несколько. И изменения конфигов тоже по куче коммитов растасканы. А посмотреть притиер в деле надо, согласен, просто давайте от этой ветки Леха новую сделает, там форматнёт и отдельным ПР сделает

chmnkh commented 5 years ago

А то этот ПР не понятно как принимать, потому что мы не видим притир в деле.

common конфиг в этом пр - это пример приттиера в деле а в остальном что еще нужно видеть и зачем? мне кажется, чтобы принять пр, надо подумать над тем, что я с линтерами сделал и что я повешал на прекоммит и препуш, устраивает ли это нас, может надо что-то переделать

а тупо проверять то как приттиер форматит - это странно, я не знаю, какую пользу из этого ты извлечь хочешь

тем более что у приттиера вариантов конфига минимум, все опции за 5 минут просматриваются

chmnkh commented 5 years ago

А если вдруг конфиг, который я сделал, окажется донным, то заформаченный по нему код тоже смысла не имеет. А так вы примете конфиг, я по нему потом все заформачу, вроде бы логично.

Znack commented 5 years ago

Притиер удобен, когда привыкнешь к формату и не будешь этому чересчур много внимания уделять, тогда сам факт, что не надо ничего вообще форматить и все само по ctrl+s будет вставать на места заметно повысит производительность :)

чт, 25 апр. 2019 г. в 0:18, Sergey Petrov notifications@github.com:

@sk1e commented on this pull request.

In package.json https://github.com/fullstack-development/react-redux-starter-kit/pull/111#discussion_r278232505 :

@@ -283,5 +289,16 @@ "typescript": "^3.2.2", "url-loader": "^1.1.2", "uuid": "^3.3.2"

  • },
  • "husky": {
  • "hooks": {
  • "pre-commit": "lint-staged",

а как ты определяешь что ты лучше форматируешь)?

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

я думаю тут нет какого то золотого стандарта, смысл преттиера в унификации

мне кажется эта либа близка, но она не различает некоторые категории и всё под одну гребёнку форматирует.

Унификация полезна, поэтому хотелось бы иметь обсуждение в ишью, с возможностью высказать свою позицию.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fullstack-development/react-redux-starter-kit/pull/111#discussion_r278232505, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWLCRCSJ57GAT2HUWQKSNLPSCI5BANCNFSM4HFDSZSA .

in19farkt commented 4 years ago

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

Пример, у меня запущен линтер и тесты в вотч режиме, я внес изменения в код, хочу коммитнуть и продолжить работу, но чтобы это сделать в обязательном порядке нужно прогнать все тесты и линтеры, которые прямо сейчас висят в вотч режиме. Прогон тестов насколько мы знаем может занимать не мало времени. В итоге на каждый коммит разработчик идет курить, пить чай и т.п.

Да, прогон тестов и линтинга важен, но это нужно делать не локально на коммит или пуш, а настраивать в CI какой-нибудь, и возможно даже не для всех веток, а только для пулл реквестов.. Это всё довольно легко настраивается. Можно настроить чтобы в мастер напрямую коммитить нельзя было, а только через ПРы, в которых все подключенные CI инструменты отработали без ошибок.

in19farkt commented 4 years ago

@Znack предлагаю закрыть этот ПР, проще это сделать заново