Skip to content
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

Refactors files_external app commands #39131

Merged

Conversation

fsamapoor
Copy link
Member

@fsamapoor fsamapoor commented Jul 4, 2023

Summary

I have made some adjustments to the apps/files_external/lib/Command classes to improve code readability.

The improvements in this PR include but are not limited to:

  • Using PHP8's constructor property promotion
  • Adding return types
  • Replacing return code integers with more readable strings.
  • Using early returns

Checklist

apps/files_external/lib/Command/Import.php Fixed Show fixed Hide fixed
}

private function createStorage(StorageConfig $mount): IStorage {
$class = $mount->getBackend()->getStorageClass();
return new $class($mount->getBackendOptions());
}

private function markParentAsOutdated($mountId, $path, OutputInterface $output, bool $dryRun) {
private function markParentAsOutdated($mountId, $path, OutputInterface $output, bool $dryRun): void {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $mountId has no provided type
}

private function createStorage(StorageConfig $mount): IStorage {
$class = $mount->getBackend()->getStorageClass();
return new $class($mount->getBackendOptions());
}

private function markParentAsOutdated($mountId, $path, OutputInterface $output, bool $dryRun) {
private function markParentAsOutdated($mountId, $path, OutputInterface $output, bool $dryRun): void {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $path has no provided type
$backend = $storage->getBackend();
$backend->manipulateStorageConfig($storage);
}

private function updateStorageStatus(StorageConfig &$storage, $configInput, OutputInterface $output) {
private function updateStorageStatus(StorageConfig &$storage, $configInput, OutputInterface $output): void {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $configInput has no provided type
@solracsf solracsf added 3. to review Waiting for reviews technical debt labels Jul 5, 2023
@solracsf solracsf added this to the Nextcloud 28 milestone Jul 5, 2023
@solracsf
Copy link
Member

solracsf commented Jul 5, 2023

CI and Psalm failing 🔴

@fsamapoor
Copy link
Member Author

CI and Psalm failing 🔴

Psalm's not failing. Is it?
It just shows some notices for which I'm not sure if I should make the required adjustment in the scope of this PR.

Other failing actions do not seem to be related to the changes that I have made and none of them are labeled as "Required".
Could you point out any specific failing actions that I can fix?

@szaimen szaimen requested review from skjnldsv, CarlSchwan, come-nc, a team and ArtificialOwl and removed request for a team July 12, 2023 12:08
apps/files_external/lib/Command/Import.php Outdated Show resolved Hide resolved
@fsamapoor fsamapoor force-pushed the refactor_lib_files_external_commands branch from 437d0b1 to 86822ee Compare September 28, 2023 05:30
fsamapoor and others added 2 commits September 28, 2023 09:02
To improve code readability.

Signed-off-by: Faraz Samapoor <[email protected]>
Signed-off-by: Faraz Samapoor <[email protected]>
Co-authored-by: Côme Chilliet <[email protected]>
Signed-off-by: Faraz Samapoor <[email protected]>
Signed-off-by: Faraz Samapoor <[email protected]>
@fsamapoor fsamapoor force-pushed the refactor_lib_files_external_commands branch from 86822ee to eab7818 Compare September 28, 2023 05:32
@fsamapoor fsamapoor requested a review from artonge September 28, 2023 05:34
@artonge
Copy link
Contributor

artonge commented Sep 28, 2023

CI failure unrelated

@artonge artonge merged commit 45cac16 into nextcloud:master Sep 28, 2023
48 of 51 checks passed
@fsamapoor fsamapoor deleted the refactor_lib_files_external_commands branch September 28, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants