-
Notifications
You must be signed in to change notification settings - Fork 731
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
Improve Matrix class #5887
Improve Matrix class #5887
Conversation
fun getUserAgent() = userAgentHolder.userAgent | ||
|
||
/** | ||
* Return the AuthenticationService | ||
*/ | ||
fun authenticationService() = authenticationService |
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.
not for this PR, we have a mix of get${object}
and ${object}
, is it something we want to align on in the public api?
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.
Yes, we have a mix, the (un-official!) rule so far is to not use the get
prefix for the SDK services, and I think this is the case for all services, to avoid having long name for the methods.
Happy to discuss this more, and/or to change the rule.
I have renamed workerFactory()
because this is not a SDK service.
997e51a
to
0ed647d
Compare
thanks for the review. I have force pushed to fix merge conflict, also added a last commit to fix Kdoc issue. |
Matrix SDKIntegration Tests Results:
|
I think this has caused a regression in the integration tests (this also happened to me locally) - looks like we've changed how we instantiate the TestMatrix, and the android WorkManagerImpl is complaining.
|
Might work (so using the unit test delegate thing to handle it) to avoid repeated initialization BUT, it's still a global and I think this means we need to prevent tests running in parallel otherwise race conditions will happen. (trying this locally to see how it fares) |
Thanks for your investigation |
Small change in the Matrix class: deprecated methods have been removed and the constructor is now public. Also the fun
workerFactory()
has been renamed togetWorkerFactory()
Documentation has been improved to.
Knit has been set up, also on the CI. For now we are not doing a big usage of it, but can be more useful in the future.
Also generated documentation header will have working link:
(see previous version without links in #5854)