Refactor lib/private #39249
Closed
Refactor lib/private #39249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Could you please resolve the conflicts? |
Signed-off-by: Hamid Dehnavi <[email protected]>
Signed-off-by: Hamid Dehnavi <[email protected]>
shdehnavi
force-pushed
the
refactor_lib_private_root_classes_part_3
branch
from
083a390
to
f837b57
Compare
Merged
This was referenced Mar 12, 2024
Merged
Merged
Merged
skjnldsv
force-pushed
the
refactor_lib_private_root_classes_part_3
branch
from
be88bf5
to
c3ca53f
Compare
skjnldsv
approved these changes
May 2, 2024
skjnldsv
force-pushed
the
refactor_lib_private_root_classes_part_3
branch
from
c3ca53f
to
876fbbe
Compare
Signed-off-by: John Molakvoæ <[email protected]>
skjnldsv
force-pushed
the
refactor_lib_private_root_classes_part_3
branch
from
876fbbe
to
98ef21f
Compare
come-nc
requested changes
May 6, 2024
| */ | ||
| protected function getEnabledDefaultProvider() { | ||
| protected function getEnabledDefaultProvider(): ?array { |
There was a problem hiding this comment.
Suggested change
| protected function getEnabledDefaultProvider(): ?array { | |
| protected function getEnabledDefaultProvider(): array { |
I do not see where it returns null?
Docblock @return should be reverted to array as well or removed.
| private SystemConfig $config; | ||
|
|
||
| private IEventLogger $eventLogger; | ||
| private \Redis|\RedisCluster $instance; |
There was a problem hiding this comment.
This cannot work because the property is not instanciated in constructor.
Most likely should be changed to:
Suggested change
| private \Redis|\RedisCluster $instance; | |
| private \Redis|\RedisCluster|null $instance; |
And then the class should be adapted so that psalm is happy. The create method is used for creating the object.
The instanceof test in getInstance looks wrong as it does not test cluster.
| @@ -1446,23 +1445,23 @@ public function boot() { | |||
| * @return \OCP\Calendar\IManager | |||
| * @deprecated 20.0.0 | |||
| */ | |||
| public function getCalendarManager() { | |||
| public function getCalendarManager(): \OCP\Calendar\IManager { | |||
There was a problem hiding this comment.
Not sure it makes sense to strong type these obsolete methods.
There was a problem hiding this comment.
deprecated since 20 means we can even remove it when not actively used in shipped apps or server :P
| return $this->get(CsrfTokenManager::class); | ||
| } | ||
|
|
||
| /** | ||
| * @return IThrottler | ||
| * @deprecated 20.0.0 | ||
| */ | ||
| public function getBruteForceThrottler() { | ||
| public function getBruteForceThrottler(): Throttler { |
There was a problem hiding this comment.
Suggested change
| public function getBruteForceThrottler(): Throttler { | |
| public function getBruteForceThrottler(): IThrottler { |
skjnldsv
added
2. developing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The required adjustments have been made to the following classes under
/lib/privatenamespace:PreviewManager.phpRepair.phpRedisFactory.phpSearch.phpServer.phpServerContainer.phpThe improvements:
if (isset(...))to null coalescing operatorChecklist