Skip to content
This repository was archived by the owner on Jul 6, 2023. It is now read-only.

Delivery of 4.2#2

Open
macheitor wants to merge 24 commits into
mainfrom
4.2
Open

Delivery of 4.2#2
macheitor wants to merge 24 commits into
mainfrom
4.2

Conversation

@macheitor

Copy link
Copy Markdown
Owner

No description provided.

@macheitor macheitor changed the title entrega ejercicio 4.2 Delivery of 4.2 May 26, 2022
@StratocasterO

Copy link
Copy Markdown

Vale, Víctor! Molt bona feina! Ahí van alguns comentaris:

  • Només he trobat una ruta que no retorna un json: la del TOKEN ❗
  • Al editar el nom d'un jugador, si es anònim, canvia tots els anònims ❗
  • Quan no hi ha dades a la base de dades retornes 200 i el json amb l'array buit. Estaria guai retornar un no content amb un missatge al json en comptes d'això
  • Molt ben fet amb l'estructura. Només tens alguna cosa amb nom genèric, rollo middlewares.js, que podria tenir un nom més descriptiu del que fan
  • Tens una mica massa logs a la consola, no hauries d'ensenyar el resultat de les queries i això. La consola és només per okays i errors ❌
  • Ben fet també amb la col·lecció de Postman, però no és bona pràctica posar més d'una vegada la mateixa ruta (com la que fas amb diferents noms). Mentre la documentació estigui clara, una request per endpoint i l'usuari ja canviarà coses per fer proves

Bona feina en general, corregeix aquestes coses i el tens força polit! Perdona pel retard al corregir, que he anat de cul aquests dies. Ara et dono el vist i plau per projecte i aviso perquè et posin a DS 🔥

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants