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

[BUGFIX] #24: exception when user name and/or password is incorrect in Frontend #25

Merged
merged 2 commits into from
Oct 17, 2018

Conversation

dmitryd
Copy link
Contributor

@dmitryd dmitryd commented Oct 11, 2018

Fixes the issue by checking that variable is set to a proper object.

@liayn
Copy link
Member

liayn commented Oct 11, 2018

Since version 8 the TimeTracker is a singleton, so we should adjust this to use makeInstance if the global TT is not set

@dmitryd
Copy link
Contributor Author

dmitryd commented Oct 16, 2018

Why should we create TT if it does not exist? If it does not exist, it means nobody is logging anything. Not worth creating it than.

@liayn
Copy link
Member

liayn commented Oct 16, 2018

The TT is not set at all anymore since v8. Checking the global only means that this logging will only happen in v7.
Therefore I would create the TimeTracker.

But I have no hard feelings about this, we can also simply remove the TT stuff completely.

@dmitryd
Copy link
Contributor Author

dmitryd commented Oct 17, 2018

I would prefer to remove it. I am not sure if I added it and if I did, why. May be, it was when the ext was originally programmed for 4.x, and TYPO3 did not have any other logging ways.

It could make sense to use a regular logger these days but than EXT:openid will show false warnings in logs every time when password was entered incorrectly. So the best would be to remove it. I will update the PR.

@dmitryd
Copy link
Contributor Author

dmitryd commented Oct 17, 2018

To the maintainer: please, squash when merging. Thanks!

@liayn liayn merged commit f819f58 into FriendsOfTYPO3:master Oct 17, 2018
@dmitryd dmitryd deleted the issue_24 branch October 17, 2018 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants