Codecov Report

Merging #40 into master will increase coverage by 20.02%. The diff coverage is 78.87%.

@@             Coverage Diff             @@
##           master      #40       +/-   ##
===========================================
+ Coverage   60.88%   80.91%   +20.02%     
===========================================
  Files          46       38        -8     
  Lines         317      241       -76     
  Branches       31       29        -2     
===========================================
+ Hits          193      195        +2     
+ Misses        114       40       -74     
+ Partials       10        6        -4

Impacted Files	Coverage Δ
src/Seguidor/TreeView/SubjectYears.js	`100% <ø> (ø)`	:arrow_up:
src/Seguidor/CarouselView/SubjectYears.js	`100% <ø> (ø)`	:arrow_up:
src/Errors/NotFound.js	`100% <ø> (ø)`
src/App/Home.js	`100% <ø> (ø)`
src/store/configureStore.js	`0% <0%> (ø)`
src/Routes/AppRoute.js	`0% <0%> (ø)`
src/App/App.js	`100% <100%> (ø)`
src/Seguidor/YearOfStudy/YearOfStudy.js	`100% <100%> (ø)`
src/Fetching/FetchingIndicator.js	`100% <100%> (ø)`
src/Seguidor/Pendientes/Pendientes.js	`100% <100%> (+75%)`	:arrow_up:
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6efd4e1...d5919b5. Read the comment docs.

gndelia commented 5 years ago

porque migramos a hooks? Justamente lo que recomienda el team de react es no ir como loco a migrar todo a hooks.

fdemian commented 5 years ago

¿Eh, donde dicen eso, Gonza? Hasta donde entiendo dicen que no es necesario migrar inmediatamente, pero que conviene. No me parece que cambiar el par (casi literalmente, son 2 o 3) componente cuente como "apurarnos a migrar". Nosotros, justamente tenemos pocos componentes y nos podemos permitir hacer el cambio. El futuro llego hace rato y son hooks (todo un palo, ya lo se).

El cambio a hooks es, justamente porque son funciones en lugar de clases (las clases en JS son un quilombo internamente). Además:

Las funciones son más fáciles de sestear.
Podés reusar logica entre componentes.
Son componentes más simples que usar clases.
Tienen ventajas para el code splitting.
Hacen que sea más fácil el trabajo para los minificadores de código.

¿Cual era tu preocupación mas allá de eso respecto a hooks?

gndelia commented 5 years ago

Crucially, Hooks work side-by-side with existing code so you can adopt them gradually. There is no rush to migrate to Hooks. We recommend avoiding any “big rewrites”, especially for existing, complex class components. It takes a bit of a mindshift to start “thinking in Hooks”. In our experience, it’s best to practice using Hooks in new and non-critical components first, and ensure that everybody on your team feels comfortable with them. After you give Hooks a try, please feel free to send us feedback, positive or negative.

emphasis mine

no te lo voy a borrar si ya lo hiciste jajaj pero creo que capaz se podian hacer otras cosa (?)

fdemian commented 5 years ago

"Big rewrites" no creo que aplique 😄 .

Por eso dije que en una codebase más grande sería un issue. Acá no me parece realmente que sea el caso. Si tenés alguna incomodidad con usar hooks lo vamos viendo, pero lo otro, con solo 3 componentes "serios" que realmente usan clases (y que fueron los únicos refactoreados para hooks), no me parece muy aplicable.

PD: por las dudas, hice testing y la aplicación no tiene nuevos errores con esto.

fdemian commented 5 years ago

Hice un par de commits mas donde le meti un poco de amor a la UI de mobile en el visor de materias (no necesitamos una "vista compacta" al estar en mobile, ya que es la única que hay ahi). Hacen falta más retoques igual.

@gndelia , después abrimos thread para esto si queréis, pero...tenías objeciones al hecho de usar hooks? O simplemente te parece que el cambio fue muy rápido?

Javier-Rotelli commented 5 years ago

yo si tengo objeciones a usar hooks. en lineas generales se puede lograr lo mismo con recompose. y no te quedan los componentes super acoplados e intesteables. en fin. move on. hay mil cosas que objetaria aca. pero me parece mejor si le damos para adelante

gndelia commented 5 years ago

ojo que el de recompose ya empezó a avisar que quiere discontinuarlo y recomendar usar hooks

parece que nadie se quiere perder ese tren 🤷‍♂

Javier-Rotelli commented 5 years ago

No es tan asi: https://github.com/acdlite/recompose/issues/756#issuecomment-438674573 en fin. en mi experiencia hooks te acopla todo de manera grosera. pero 🤷‍♂

UTNianos / frontend

Modernize code #40

Codecov Report