-
Notifications
You must be signed in to change notification settings - Fork 5
feat: use lazy app config values and small fixes #191
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
base: main
Are you sure you want to change the base?
Conversation
kyteinsky
left a comment
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.
a few confusion points here. It'd nice to have a piece of your mind on them.
32e3702 to
2f19e88
Compare
0cb32e7 to
bca71e7
Compare
- use `isEnabledForAnyone` instead of `isInstalled` to check for app_api - type annotations for both OCP and the OCA IContentProvider - make `indexed_files_count` app config non-lazy Signed-off-by: Anupam Kumar <[email protected]>
- use `isEnabledForAnyone` instead of `isInstalled` to check for app_api - type annotations for both OCP and the OCA IContentProvider - make `indexed_files_count` app config non-lazy Signed-off-by: Anupam Kumar <[email protected]>
Signed-off-by: Anupam Kumar <[email protected]>
Signed-off-by: Anupam Kumar <[email protected]>
bca71e7 to
9976be0
Compare
| } | ||
|
|
||
| $qb = $this->connection->getQueryBuilder(); | ||
| $qb->update('appconfig') |
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.
Mmh, why are you bypassing the Appconfig interface? The db table is not guaranteed to be stable, while the OCP interface is
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.
it would be hard to guess if the config was originally set or not so we might end up setting it
isEnabledForAnyoneinstead ofisInstalledto check for app_api