- Issue created by @jelle_s
- @jelle_s opened merge request.
- Status changed to Needs work
over 1 year ago 8:43pm 10 May 2023 - 🇧🇪Belgium matthijs
Thanks for your MR. That's a useful feature indeed, but I added some remarks/toughts to the MR.
- 🇧🇪Belgium jelle_s Antwerp, Belgium
Thanks for the review, added my remarks as well. I'll update the MR after your input.
- Status changed to Needs review
over 1 year ago 9:43am 16 May 2023 - 🇧🇪Belgium jelle_s Antwerp, Belgium
I updated the MR based on some of the review remarks.
- Status changed to Needs work
over 1 year ago 12:18pm 16 May 2023 - Status changed to Needs review
over 1 year ago 2:36pm 16 May 2023 - 🇧🇪Belgium matthijs
Changed some class names, added an interface for the validator and fixed some code styles issues.
- 🇧🇪Belgium jelle_s Antwerp, Belgium
Looks good to me.
Nitpic: In a couple of docblocks you wrote "authantication" instead of "authentication".
- 🇧🇪Belgium matthijs
Fixed, thanks for mentioning! Will merge asap, but wanted to do some testing first. Unless you tested it already?
- 🇧🇪Belgium jelle_s Antwerp, Belgium
I've tested the default functionality: logging in with an already mapped account, and with an account that wasn't mapped in authmap yet, both worked as expected. Logout still works as expected too. I haven't tested writing a custom event listener yet, but since the default implementation is an event listener as well, I don't expect any problems there.
-
Matthijs →
committed 73e48a11 on 2.x authored by
Jelle_S →
Issue #3358166 by Jelle_S, Matthijs: DX: Allow devs to alter loading...
-
Matthijs →
committed 73e48a11 on 2.x authored by
Jelle_S →
- Status changed to Fixed
over 1 year ago 3:13pm 26 May 2023 Automatically closed - issue fixed for 2 weeks with no activity.