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
fix(authenticator): improve performance #1914
Conversation
@irinaschubert @mpro7 could you please try this branch and see if the |
@subotic I got numerous errors while synchronising which is probably related to failed build, but all tests passed in |
I did some refactoring and missed a spot that needs changing. I will fix that and then you can try again. |
@irinaschubert @mpro7 could you please try again and see if this solves the issue with the one test? |
I tested it locally and the tests run through now. |
@subotic this time both tests and syncing ended w/o errors 👍 |
|
||
/** | ||
* Not implemented. | ||
*/ | ||
def read(jsonVal: JsValue) = ??? | ||
def read(jsonVal: JsValue): PermissionProfileType = ??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this isn't implemented should be here at all or at least uncommented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question. I can comment it out. It is not used anywhere in the code, or it would throw an exception when used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do that in my branch, found few more occurrences with ???
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or not, because it's inherited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 16 occurrences with ???
inside the whole codebase. If the code is not used anywhere, then the not existing implementation doesn't hurt.
resolves DEV-78