Skip to content

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented May 15, 2025

The diff is not that huge.

A few things could be improved, there are useless @var annotations, and also weird constructors intead of proper DI usage.

@come-nc come-nc self-assigned this May 15, 2025
@provokateurin
Copy link
Member

A few things could be improved, there are useless @var annotations, and also weird constructors intead of proper DI usage.

Those are easily fixable with rector :)

Comment on lines 103 to +114
$this->userManager = $userManager;
$this->rootFolder = $rootFolder;
$this->l10n = $l10n;
$this->logger = OC::$server->get(LoggerInterface::class);
$this->logger = Server::get(LoggerInterface::class);
$this->urlGenerator = $urlGenerator;

$this->federatedUserService = OC::$server->get(FederatedUserService::class);
$this->federatedEventService = OC::$server->get(FederatedEventService::class);
$this->shareWrapperService = OC::$server->get(ShareWrapperService::class);
$this->shareTokenService = OC::$server->get(ShareTokenService::class);
$this->circleService = OC::$server->get(CircleService::class);
$this->eventService = OC::$server->get(EventService::class);
$this->federatedUserService = Server::get(FederatedUserService::class);
$this->federatedEventService = Server::get(FederatedEventService::class);
$this->shareWrapperService = Server::get(ShareWrapperService::class);
$this->shareTokenService = Server::get(ShareTokenService::class);
$this->circleService = Server::get(CircleService::class);
$this->eventService = Server::get(EventService::class);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all be promoted properties from DI.

Comment on lines 41 to 47
public function __construct() {
parent::__construct(
OC::$server->get(IDBConnection::class),
OC::$server->get(SystemConfig::class),
OC::$server->get(LoggerInterface::class)
Server::get(IDBConnection::class),
Server::get(SystemConfig::class),
Server::get(LoggerInterface::class)
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?
Let’s remove this constructor?

@come-nc come-nc marked this pull request as ready for review May 15, 2025 15:13
@come-nc
Copy link
Contributor Author

come-nc commented May 15, 2025

The rest can be in follow-ups

@come-nc come-nc requested a review from ArtificialOwl May 15, 2025 15:14
@come-nc come-nc merged commit 632ff1e into master May 15, 2025
32 of 33 checks passed
@come-nc come-nc deleted the fix/add-rector branch May 15, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants