Kommentarer

[1dv450]

Hej! Ursäkta den sena feedbacken men här kommer i alla fall lite kommentarer.

På det hela taget tycker jag det ser bra ut. Du verkar förstått konceptet och byggt ett godkänt API. Jag har några kommentarer och tips förutom de din peer nämnt. Kanske saker mer att tänka på än något du direkt behöver ändra.

• Har du satt användare 1dv450 som collaborator till ditt repo?
• Kan kanske tycka du är lite snål med HATEOAS i vissa sammanhang. T.ex. finsn ingen information elelr länk till skaparen av ett "place" när man efterfrågar dessa. Kanske är ett medvetet val. Men att ge en länk till "placets" "creator" vore inte fel.
• Du använder en Authorization-header för att skicka din ApiNyckel. Jag vet att många gör det och det är taget efter en film. Dock man diskutera detta då den headern är mer tänkt för just Autentisering och Aktorisering medans en API-nyckel mer är en identifikation av applikationen. Jag hade nog istället lagt min JWT i den headern och sedan haft en custom Header så som X-Apikey eller liknande. Inget fel men något och fundera på.
• Till den slutliga peer-review får du gärna koppla dina postmananrop till din seed så slipper man ändra inloggningsuppgifter och annat...allt för att underlätta för oss som testar :)
• Errorhanteringen i dina controllers skulel man kanske kunna få lite mindre dry med en refaktoring.
• Skapande av tags kan man fundera lite kring. Nu skickar du inte med några tags vid ditt anrop för skapande av en placeresurs. Men frågan är hur de skapas. Säg att jag skapa två olika resurser med samma taggnamn. Kommer inte dessa taggar då bli två helt olika resurser i systemet (ha olika id:n ) och på så sätt blir det svårt att filtrera ut alla places som hör till en specifik tag?
• Märkte att jag fick en 400 tillbaka vid felaktig JWT. Kanske borde vara en 403 då man försöker göra något utan tillåtelse (felaktig JWT)
• Implementeringen av JWT känns ju igen ;) Du har även fått med liet för mycket kod från mitt exempel som blir onödig. Den bör du rensa upp!

Det blev många kommentarer men det är mest feedback och tankar. I det stora hela tycker jag det ser bra ut! Ska bli kul att se din klientapplikation!

[vl222cu]

Hej John,

Det är ingen fara och tack för din feedback! Jag missade helt att lägga till 1dv450 som collab i detta repo, ber så mycket om ursäkt för det, bra ändå att du lyckades hitta mitt repo :)

Jag ska se över de punkter du tagit upp och se om jag kan fixa till APIet nu medan jag jobbar på klientapplikationen :)

Hälsningar Vivi

Den 13 mars 2015 09:27 skrev John H notifications@github.com:

Hej! Ursäkta den sena feedbacken men här kommer i alla fall lite kommentarer.

På det hela taget tycker jag det ser bra ut. Du verkar förstått konceptet och byggt ett godkänt API. Jag har några kommentarer och tips förutom de din peer nämnt. Kanske saker mer att tänka på än något du direkt behöver ändra.

-

Har du satt användare 1dv450 som collaborator till ditt repo?

Kan kanske tycka du är lite snål med HATEOAS i vissa sammanhang. T.ex. finsn ingen information elelr länk till skaparen av ett "place" när man efterfrågar dessa. Kanske är ett medvetet val. Men att ge en länk till "placets" "creator" vore inte fel.

Du använder en Authorization-header för att skicka din ApiNyckel. Jag vet att många gör det och det är taget efter en film. Dock man diskutera detta då den headern är mer tänkt för just Autentisering och Aktorisering medans en API-nyckel mer är en identifikation av applikationen. Jag hade nog istället lagt min JWT i den headern och sedan haft en custom Header så som X-Apikey eller liknande. Inget fel men något och fundera på.

Till den slutliga peer-review får du gärna koppla dina postmananrop till din seed så slipper man ändra inloggningsuppgifter och annat...allt för att underlätta för oss som testar :)

Errorhanteringen i dina controllers skulel man kanske kunna få lite mindre dry med en refaktoring.

Skapande av tags kan man fundera lite kring. Nu skickar du inte med några tags vid ditt anrop för skapande av en placeresurs. Men frågan är hur de skapas. Säg att jag skapa två olika resurser med samma taggnamn. Kommer inte dessa taggar då bli två helt olika resurser i systemet (ha olika id:n ) och på så sätt blir det svårt att filtrera ut alla places som hör till en specifik tag?

Märkte att jag fick en 400 tillbaka vid felaktig JWT. Kanske borde vara en 403 då man försöker göra något utan tillåtelse (felaktig JWT)

Implementeringen av JWT känns ju igen ;) Du har även fått med liet för mycket kod från mitt exempel som blir onödig. Den bör du rensa upp!

Det blev många kommentarer men det är mest feedback och tankar. I det stora hela tycker jag det ser bra ut! Ska bli kul att se din klientapplikation!

— Reply to this email directly or view it on GitHub https://github.com/vl222cu/1DV450_vl222cu_API/issues/4.