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

Feature/eslint es2015 #84

Merged
merged 8 commits into from
Feb 28, 2016
Merged

Feature/eslint es2015 #84

merged 8 commits into from
Feb 28, 2016

Conversation

guumaster
Copy link
Contributor

This includes eslint:recommended and several extra rules to improve code quality, readability and lot more. I've tried to do it in specific commits but it was too hard so finally there are only 8 commits.

I've used eslint-nibble tool to fix one rule at a time. I Hope you like the result, I've put lots of hours to finish it.

@luismesas
Copy link
Member

ha bajado la cobertura de codigo, perono has tocado ningun fuente no?

@luismesas
Copy link
Member

dejo abierta la barra libre para bajar la cobertura de codigo hasta que lleguemos a una version sobre la que empezar a construir, yo mientras estoy cambiando como se utilizan los config

@guumaster
Copy link
Contributor Author

La bajada de cobertura es raro. En principio es solo limpieza, pero bastante profunda. Antes de mergear habria que subir mas de 80% todavia, para estar mas tranquilo.

Queria enviar el PR para trabajar no diverger demasiado y que sea imposible luego mergear.

Esto es el inicio:
Imgur

Y asi queda:
Imgur

@guumaster
Copy link
Contributor Author

Si quieres antes del merge subimos mas el coverage, casi lo prefiero tambien.

@guumaster
Copy link
Contributor Author

Es mas, estoy pensando en borrar las plataformas que estan sin probar, linkedIn y google son solo copy paste de codigo que tiene muy poca covertura unitaria ni se usa en ningun proyecto.

Lo borramos asi nos enfocamos en subir la cobertura de las features que realmente usamos?

@luismesas
Copy link
Member

ok, borralas
y la de facbook esta en un lugar aparte
lo del 80% no lo veo, prefiero dejar en master una version en la que confiemos sea cual sea la cobertura y empezar a mejorar

@guumaster
Copy link
Contributor Author

La de facebook es auto loginByToken no es flujo oAuth, por eso esta en un sitio aparte. pero es probable que haya que moverlo a platform mejor.

Ok, ya por hoy para mi sufiente. Cuando tenga mas tiempo sigo.

@luismesas luismesas merged commit 7db9b61 into master Feb 28, 2016
@luismesas luismesas deleted the feature/eslint-es2015 branch February 28, 2016 23:17
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.

None yet

2 participants