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

Uses PHP8's constructor property promotion in core/Command and / #38775

Conversation

fsamapoor
Copy link
Member

Following #38636, #38637, and #38638 PRs, I have made the required adjustments to the /core/Command/ namespace as well.

I figured I should split the changes into multiple PRs to make reviewing the changes easier.

This PR refactors /core/Command and /core/Command/TwoFactorAuth classes by using PHP8's constructor property promotion to remove redundant lines and make the code cleaner.

@szaimen szaimen requested review from juliushaertl, come-nc, a team, ArtificialOwl and Fenn-CS and removed request for a team June 12, 2023 16:38
@szaimen szaimen added 3. to review Waiting for reviews technical debt labels Jun 12, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Jun 12, 2023
core/Command/TwoFactorAuth/Disable.php Outdated Show resolved Hide resolved
@artonge artonge requested a review from a team June 19, 2023 10:27
@artonge artonge force-pushed the constructor_property_promotion_in_core_command_part9 branch from 4c47df0 to ff23cb5 Compare June 19, 2023 15:44
fsamapoor pushed a commit to fsamapoor/server that referenced this pull request Jun 20, 2023
Based on:
nextcloud#38775 (comment)

Signed-off-by: Faraz Samapoor <fsa@adlas.at>
core/Command/TwoFactorAuth/Cleanup.php Outdated Show resolved Hide resolved
core/Command/TwoFactorAuth/Disable.php Outdated Show resolved Hide resolved
core/Command/TwoFactorAuth/Enable.php Outdated Show resolved Hide resolved
core/Command/TwoFactorAuth/Enforce.php Outdated Show resolved Hide resolved
core/Command/Check.php Outdated Show resolved Hide resolved
core/Command/TwoFactorAuth/State.php Outdated Show resolved Hide resolved
Copy link
Contributor

@Fenn-CS Fenn-CS left a comment

Choose a reason for hiding this comment

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

Thank you!

@fsamapoor fsamapoor requested a review from come-nc June 20, 2023 15:37
artonge pushed a commit to fsamapoor/server that referenced this pull request Jun 20, 2023
Based on:
nextcloud#38775 (comment)

Signed-off-by: Faraz Samapoor <fsa@adlas.at>
@artonge artonge force-pushed the constructor_property_promotion_in_core_command_part9 branch from 7960f14 to ee5a0b0 Compare June 20, 2023 16:47
artonge pushed a commit to fsamapoor/server that referenced this pull request Jun 21, 2023
Based on:
nextcloud#38775 (comment)

Signed-off-by: Faraz Samapoor <fsa@adlas.at>
@artonge artonge force-pushed the constructor_property_promotion_in_core_command_part9 branch from ee5a0b0 to 4d5159c Compare June 21, 2023 07:07
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

There was 1 error:

  1. Core\Command\TwoFactorAuth\CleanupTest::testCleanup
    ArgumentCountError: Too few arguments to function OC\Core\Command\TwoFactorAuth\Cleanup::__construct(), 1 passed in /drone/src/tests/Core/Command/TwoFactorAuth/CleanupTest.php on line 47 and exactly 2 expected

/drone/src/core/Command/TwoFactorAuth/Cleanup.php:36
/drone/src/tests/Core/Command/TwoFactorAuth/CleanupTest.php:47

@fsamapoor fsamapoor requested a review from artonge June 23, 2023 17:51
Faraz Samapoor and others added 7 commits June 24, 2023 23:14
 in core/Command and /TwoFactorAuth classes.

Signed-off-by: Faraz Samapoor <fsa@adlas.at>
Based on:
nextcloud#38775 (comment)

Signed-off-by: Faraz Samapoor <fsa@adlas.at>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
fsamapoor and others added 2 commits June 24, 2023 23:14
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Signed-off-by: Faraz Samapoor <fsa@adlas.at>
@artonge artonge force-pushed the constructor_property_promotion_in_core_command_part9 branch from 5cbfe2b to fd0e2f7 Compare June 24, 2023 21:14
@artonge
Copy link
Contributor

artonge commented Jun 24, 2023

@come-nc

@come-nc come-nc merged commit 783f1b9 into nextcloud:master Jun 26, 2023
39 checks passed
@fsamapoor fsamapoor deleted the constructor_property_promotion_in_core_command_part9 branch November 3, 2023 12:50
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.

None yet

5 participants