Skip to content
This repository was archived by the owner on Sep 29, 2021. It is now read-only.

Feature/ch17/create secured and unsecured routing#25

Open
mVabo wants to merge 5 commits intomainfrom
feature/ch17/create-secured-and-unsecured-routing
Open

Feature/ch17/create secured and unsecured routing#25
mVabo wants to merge 5 commits intomainfrom
feature/ch17/create-secured-and-unsecured-routing

Conversation

@mVabo
Copy link
Contributor

@mVabo mVabo commented Mar 13, 2021

No description provided.

@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #17: Create secured and unsecured routing.

@vercel
Copy link

vercel bot commented Mar 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/echokarriere/frontend/2rKuwr6TRGpiR9XCVSszLf9iZqRk
✅ Preview: Failed

@sondr3
Copy link
Contributor

sondr3 commented Mar 14, 2021

Ser stort sett greit ut, litt vanskelig å reviewe når du har to PRer som er basert på hverandre. Du også skru på strictNullChecks, hele poenget med å bruke TS er å ha disse sjekkene. Om du kunne prøvd å redusere flere av // eslint-disable så hadde det vært fint og // @ts-ignore har jeg ikke lyst til å merge inn 😄

@sondr3
Copy link
Contributor

sondr3 commented Apr 3, 2021

Jeg har sett over koden nå de sist dagene for å forstå den, og på det store og hele vet jeg ikke helt om jeg vil merge dette. Tror jeg kanskje har kommunisert issuen litt dårlig, men det er ikke noe forskjell på unauthenticated/autenticated views, og ikke forskjell heller på om man er admin eller vanlig bruker, så vi trenger ikke to forskjellige navbars/layouts osv. Jeg har aldri vært borti din måte å gjøre authorization, har du brukt den før? Har vanligvis løst det med en enkel AuthRoute som sjekker om brukeren er authenticated og har tilgang. Sorry for at det har tatt tid å review, si ifra når du er ledig så kan vi gå over det på Discord/lesesalen. 😃

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