insignia4u / quvey

Quick Survey.
0 stars 0 forks source link

[Code Review April 15th 2013] #5

Closed jmax closed 11 years ago

jmax commented 11 years ago
jmax commented 11 years ago

@manuelramos por favor, revisa estos cambios que agregué en el repo.

Un par de cuestiones a tener en cuenta...

1) https://github.com/insignia4u/quvey/blob/master/db/migrate/20130415043804_create_users.rb

Como podés ver, agregué unos índices para optimizar el diseño de la DB. Siempre es importante, al momento de agregar una nueva table, revisar qué campos van a necesitar ser indexados a fin de optimizar futuras consultas. En el caso de la tabla users, sale de cajón que necesitas un índice por la combinación provider+uid y otro por oauth_token.

Para que no tengas drama con este cambio, te recomiendo eliminar tu archivo db/development.sqlite3 y corré nuevamente la tarea rake db:migrate.

2) https://github.com/insignia4u/quvey/tree/master/spec

Reorganicé la carpeta spec. Todo lo que sea spec requests debe ir (por una práctica común) dentro de la carpeta spec/requests.

3) https://github.com/insignia4u/quvey/blob/master/spec/spec_helper.rb

Agregué unos seteos extras en spec_helper.rb a fin de que no tengamos problemas futuros con los spec requests, sobre todo.

4) Specs válidos.

Ahí agregué los siguientes specs:

https://github.com/insignia4u/quvey/blob/master/spec/requests/facebook_login_spec.rb https://github.com/insignia4u/quvey/blob/master/spec/models/user_spec.rb

Ambos están pasando y tiran un coverage del 100% sobre la app. La idea, a partir de ahora, es mantener estos specs pasando en todo momento (sin fallas) y verificar con specs cualquier feature nuevo que agreguemos.

5) Algunas recomendaciones.

Algo extremadamente importante es la prolijidad del código. Por ejemplo, en el modelo:

https://github.com/insignia4u/quvey/blob/master/app/models/user.rb

Habían espacios de más y un "end" mal tabulado. Asimismo, encontré varios lugares que tenían 2 y 3 saltos de carro al final del archivo, la buena práctica consiste en dejar siempre 1 enter al final de cada archivo.

Otro tema importante es el largo de las líneas. Idealmente, es un buen gesto mantenerse dentro de los 80 caracteres de largo para cada línea de código.

Este es un punto donde, normalmente, ponemos muchísima atención y somos medio pesados (yo sobre todo :smile: ).