Skip to content
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.

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

Linter must be fed #23

Open
Javier-Rotelli opened this issue Dec 28, 2017 · 7 comments
Open

Linter must be fed #23

Javier-Rotelli opened this issue Dec 28, 2017 · 7 comments

Comments

@Javier-Rotelli
Copy link
Member

che, abro la discusion aca porque me parece mas apropiado que en el PR

si el linter se queja, la ultima opcion es deshabilitarle los warnings, para mi es como ponerle cinta a la luz de "revise el motor".

en lineas generales prefiero que si estamos haciendo algo que no le gusta al linter lo desactivemos para esa linea/funcion/archivo en particular y no para todo.

e65cebc
856fc13

particularmente estas dos me gustaria que vuelvan a estar:
https://eslint.org/docs/rules/no-unneeded-ternary (btw e65cebc#diff-338c05c4a3a26b3ef448d5647c424a19R12 eso hace llorar al niñito jesus :P )
https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md

iba a crear un PR con esto, pero creo que primero tendriamos que ponernos de acuerdo :D

social!

@Javier-Rotelli
Copy link
Member Author

@fdemian, @gonnza

@fdemian
Copy link
Contributor

fdemian commented Dec 29, 2017

Buenas!

En particular el estilo de airbnb me parece...medio sospechoso a veces. En particular me parece que hay justificaciones válidas para comentar ciertos parámetros.

  • react/jsx-no-bind: parece ser una prop. nueva en la ultima versión de reglas de airbnb (al menos no la recuerdo de la anterior vez). Nos fuerza a a cambiar gran parte del código porque los de airbnb decidieron que usar arrow functions es "poco performante". En mi opinion implica dejar una forma muy conveniente de escribir el código por un tradeoff que no me queda claro que nos termine beneficiando.

  • jsx-a11y/anchor-is-valid : tira error con los links de react-router-dom. Podemos prescindir de los links de react router dom, acordarnos de marcar toda linea que utilize un elemento como especial o simplemente desactivar la propiedad. Una cuarta opción es esperar a que los de react-router-dom arreglen esto (o levantarles un bug). No se si es un bug de ellos igual.

  • object-curly-newline: Es algo estético que tiene el linter que no me gusto. Si querés vuelvo a poner esto en 1 (no me gusto el autofix que le hizo a una linea, quedaba muy desentonante, pero admito que esto ya es gusto personal).

  • no-unneeded-ternary: I have no strong feelings one way or the other. Lo había puesto porque segun recuerdo pasarle { isAuthenticated: false } como parametro al reducer no me estaba funcionando y quería dejarlo mockeado en false. Puedo empezar a trabajar en restaurarlo si querés, es un warning extremadamente menor que afecta solo a una linea de momento y veo que es relativamente útil.

No estoy intentando boycottear la idea del linter. De hecho, si fueran warnings los hubiera dejado todos, pero me parece que muchos terminan siendo más una molestia que una ayuda. En particular, deberíamos dejar cosas como react/jsx-no-bind como una alerta. Muchas de estas cosas generan fricción a la hora de codear. Entiendo que haya un tema con la performance en algunos casos, pero a menos que estemos en tiempos de optimizar agresivamente no me parecen prioritarias, ni el tipo de cosas que deban hacer estallar un build o impedir un merge (a menos que el impacto en performance sea sustancial o de un componente muy crítico).

Si, este post fue larguísimo.

ndf9g

PD: no, no estoy tratando de politizar el thread con la foto de obama (!). :P

@gndelia
Copy link
Contributor

gndelia commented May 1, 2019

banco la vuelta de no-unneeded-ternary y de jsx-no-bind

@fdemian
Copy link
Contributor

fdemian commented May 1, 2019

El flag no-unneeded-ternary no me parece un mal criterio. Cual es la ventaja de hacer volver jsx-no-bind? Me parece que hay razones válidas para no descatar esa manera de escribir funciones. O al menos para dejarlo como advertencia.

@Javier-Rotelli
Copy link
Member Author

Bump.
añado a la discusion. Prettier. no se si estoy dispuesto a negociarlo. hace la vida infinitamente mas facil.
also: si nadie se queja, voy a volver a habilitar no-unneded-ternary

@gndelia
Copy link
Contributor

gndelia commented Sep 30, 2019

apruebo prettier totalmente

@fdemian
Copy link
Contributor

fdemian commented Oct 8, 2019

Sin objeciones a no-unneeded-ternary. Tene en cuenta que capaz hay partes del codigo que empiezan a tirar warnings.

Prettier, creo recordar que habriamos dicho que si casi unanimemente. Mientras se pueda configurar y todos acordemos por decreto real mantener un mismo estilo, fine by me.

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

No branches or pull requests

3 participants