jojuparp / workman

2 stars 0 forks source link

Katselmointi #1

Open Jakousa opened 4 years ago

Jakousa commented 4 years ago

Katselmointi

Sivuston käytettävyys

Mitä tein

Kokemus

Koodi

Kokonaisuus

👷 / 🔨

jojuparp commented 4 years ago

Hei,

Kiitos palautteesta! Tosiaan, sovelluksesta jäi joitain toiminnallisuuksia toteuttamatta kun 3. perioidi alkoi painamaan jo päälle.

Siitä puheenollen. Osaatko sanoa onko harjoitustyötä mahdollista laajentaa vielä? Minulla vapautui hieman ylimääräistä aikaa, ja mielelläni tekisin jatkaisin vielä harjoitustyötä jos se on mahdollista.

Terv. Joni Parpala

On 21. Jan 2020, at 16.50, Jami Kousa notifications@github.com wrote:

Katselmointi

Sivuston käytettävyys

Mitä tein

21.1.2020 klo ~16:30

Kirjauduin sisään

Tein työtehtävän

Selailin sovellusta

Kokemus

Hieno sovellus!

Faviconin muuttaminen on tainnut unohtua reactin oletuksesta, tekee välilehden löytämisestä vaikeampaa.

Olisi kiva päästä tekemään oman tilin 🙂

Työtehtävän lisäyksen jälkeen voisi käyttäjän ohjata työtehtävien listaukseen tai ainakin antaa jotain feedbackiä. Esimerkiksi https://www.npmjs.com/package/react-toastify https://www.npmjs.com/package/react-toastify voisi sopia siihen.

Koodi

Tiedostot ovat hyvin nimetty ja koodi jaettu järkevästi osiin!

JobForm tiedoston sisältöä voisi ehkä jakaa pienempiin osiin. Tiedosto ei ole kovin luettava

Controllereista voisi tokenin tarkistamisen siirtää middlewareenhttps://github.com/jojuparp/workman/blob/aa2df79b21ece2c0fbfbbc80976eff3528c17fcd/server/controllers/jobs.js#L11-L18 https://github.com/jojuparp/workman/blob/aa2df79b21ece2c0fbfbbc80976eff3528c17fcd/server/controllers/jobs.js#L11-L18 Controllereista saisi myös jatkuvan try catch copypasten siirrettyä pois käyttämällä esim https://www.npmjs.com/package/express-async-errors https://www.npmjs.com/package/express-async-errors kirjastoa

Kokonaisuus

Ylläpidettävyyttä on selkeästi pidetty mielessä refaktoroinnin aikana, tästä on hyvä jatkaa.

Tehty käyttöä varten, erittäin hyvä sovellus!

👷 / 🔨

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jojuparp/workman/issues/1?email_source=notifications&email_token=AKI5V3VBFOQH2PBO5EK3UYTQ64DTRA5CNFSM4KJUWLR2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IHU7IHA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKI5V3QQFVMZRINZGARHHXDQ64DTRANCNFSM4KJUWLRQ.

Jakousa commented 4 years ago

Kyllä onnistuu, kuten maililla Luukkainen jo vastasi. Lähetä hänelle viestiä kun tulee valmiiksi 😄