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

feat(assets-rubocop-yml): enable new rules #429

Merged
merged 2 commits into from
Jan 6, 2023
Merged

Conversation

difernandez
Copy link
Contributor

@difernandez difernandez commented Jan 5, 2023

Se habilitan algunas reglas nuevas en rubocop:

Esta última me gustaría que se revise particularmente. Se define un orden en el que van los distintos elementos de una clase, pensado principalmente para el orden en modelos. El orden es una combinación del ejemplo que sale en los docs y lo que vi en platanus revisando modelos. Ahí díganme si les hace sentido o cambiarían algo.
El cop define una lista en ExpectedOrder que puede tener: grupos definidos en Categories, macros sueltas (enum, aasm), algunas cosas definidas de antemano en algún lado que rubocop entiende (constants, methods), y cosas anteriores con prefijo de nivel de privacidad.

Al recibir una clase con el orden dado vuelta lanza los siguientes errores:
image

Y al correr el autocorrect queda así:
image
Los espaciados quizás es mejorable, pero hace harto de la pega

@difernandez difernandez requested review from gmq and ldlsegovia January 5, 2023 15:56
Copy link
Contributor

@gmq gmq left a comment

Choose a reason for hiding this comment

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

Las reglas nuevas se ven útiles. No tengo opinión sobre el orden, pero me gusta que haya uno y que sea autofixable.

@difernandez
Copy link
Contributor Author

@gmq agregué una regla que me había faltado desde que revisaste por si acaso, la de redundant validation

@difernandez difernandez merged commit 58c1818 into master Jan 6, 2023
@difernandez difernandez deleted the new-rubocop-rules branch January 6, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants