-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix latercera portada #625
base: master
Are you sure you want to change the base?
Conversation
} | ||
} else { | ||
sendPortadaDate(res, fecha) | ||
res.http(testUrl).timeout(2000).get()((err, response, body) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No soy muy fan del estilo, pero la indentación debería ser igual al archivo original
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entonces prettier no está configurado en el proyecto, no? @gmq
prettier version
@@ -122,6 +130,6 @@ | |||
"husky": "0.14.3", | |||
"lint-staged": "9.4.2", | |||
"nock": "9.2.6", | |||
"prettier": "1.12.1" | |||
"prettier": "2.0.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Un version bump debería estar en un PR aparte
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
De hecho este cambio completo debería ser un PR aparte, no es de latercera este cambio?
case 302: | ||
ready = false | ||
sendPortadaDate(res, fecha) | ||
resolve(testUrl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En este caso, debería devolver el header Location
que devuelve el servidor. Eso sí, hay que chequear que Location != testUrl
para evitar caer en un bucle infinito (si son iguales, debería indicar que no hay portada)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Hay que separar los pull requests (uno por las portadas, otro por el prettifier)
- Ya que hay un procesado de código HTTP 302, debe devolver la dirección a la que redirecciona y no la original
prettier@2.0.5: | ||
version "2.0.5" | ||
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.0.5.tgz#d6d56282455243f2f92cc1716692c08aa31522d4" | ||
integrity sha512-7PtVymN48hGcO4fGjybyBSIWDsLU4H4XlvOHfq91pz9kkGlonzwTfYkaIEwiRg/dAJF9YlbsduBAgtYLi+8cFg== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esta magia debería estar en un PR aparte
fde96ef
to
d12d3d6
Compare
Descripción
Ejemplo de comportamiento