Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be able to register, create a user and login #3

Merged

Conversation

@reyostallenberg
Copy link
Member

commented Oct 2, 2019

No description provided.

@reyostallenberg reyostallenberg requested a review from gisostallenberg Oct 2, 2019
Copy link
Member

left a comment

You might consider adding a UserCreator service. You could make it LoggerAware and set a ConsoleLogger in the console command and have a NullLogger by default. This will centralize the dispatching of the events and avoids code duplication.

"phpstan analyse --level=7 src/ tests/",
"composer-require-checker",
"security-checker security:check"
Comment for lines 70  – 72

This comment has been minimized.

Copy link
@gisostallenberg

gisostallenberg Oct 2, 2019

Member

A failure in phpstan leads to not executing the other script. Because phpstan is the most likely to fail I suggest another order of this scripts

@@ -14,13 +14,19 @@
class Configuration implements ConfigurationInterface
{
const CONFIG_ROOT_KEY = 'connectholland_user';

This comment has been minimized.

Copy link
@gisostallenberg

gisostallenberg Oct 2, 2019

Member

You can make this private

@reyostallenberg reyostallenberg merged commit 573f57d into ConnectHolland:master Oct 2, 2019
@reyostallenberg reyostallenberg deleted the reyostallenberg:create-and-login-user branch Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.