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

Evaluate $upgrading numeric value when checking if maintenance mode is active #22

Merged
merged 12 commits into from
Nov 6, 2023

Conversation

selul
Copy link
Contributor

@selul selul commented Jun 2, 2022

Attempt to fix #19 by checking the upgrading value when seeing if the site is in maintenance mode or not. Re-use the logic that core uses -> https://github.com/WordPress/WordPress/blob/master/wp-includes/load.php#L310-L314

…hecking if the site is in maintenance mode or not.
@selul selul mentioned this pull request Jun 2, 2022
features/maintenance-mode.feature Outdated Show resolved Hide resolved
src/MaintenanceModeCommand.php Outdated Show resolved Hide resolved
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

I'd suggest we proceed forward on this path: #22 (comment)

We'll want to make sure we have a few different scenarios covering each of the potential .maintenance file formats.

@selul
Copy link
Contributor Author

selul commented Nov 4, 2023

Hey guys,

Thanks for the feedback; I thought to give it another try by adding the warning when the value is a non-numeric one.

Also, I've changed the regex to account for when space might be missing between the = or before ; and tried to add a few tests to cover this.

features/maintenance-mode.feature Show resolved Hide resolved
features/maintenance-mode.feature Outdated Show resolved Hide resolved
features/maintenance-mode.feature Outdated Show resolved Hide resolved
features/maintenance-mode.feature Outdated Show resolved Hide resolved
features/maintenance-mode.feature Outdated Show resolved Hide resolved
@danielbachhuber
Copy link
Member

@selul It's looking great! Just a few small things to fix up, and then this should be good to land. You'll want to make sure all of the tests are passing too.

selul and others added 4 commits November 4, 2023 15:27
Co-authored-by: Daniel Bachhuber <daniel@bachhuber.co>
Co-authored-by: Daniel Bachhuber <daniel@bachhuber.co>
Co-authored-by: Daniel Bachhuber <daniel@bachhuber.co>
@selul
Copy link
Contributor Author

selul commented Nov 4, 2023

Thanks for the feedback @danielbachhuber 🚀 !

I've applied the changes.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Looks pretty solid! Just two small tweaks remaining on my end. I'd love to get another @wp-cli/committers review, though.

features/maintenance-mode.feature Outdated Show resolved Hide resolved
src/MaintenanceModeCommand.php Show resolved Hide resolved
@selul
Copy link
Contributor Author

selul commented Nov 6, 2023

Thanks for the feedback, @danielbachhuber! I've made the changes!

@danielbachhuber danielbachhuber added this to the 2.1.0 milestone Nov 6, 2023
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this, @selul !

@danielbachhuber danielbachhuber changed the title Ensure maintenance mode consistency Nov 6, 2023
@danielbachhuber danielbachhuber merged commit 2e9845a into wp-cli:main Nov 6, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment