Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Provide WP version in the inquiry. #239

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

tomalec
Copy link
Contributor

@tomalec tomalec commented Sep 10, 2021

Fixes #157.

Description of the Change

As a WP plugin developer, I'd like to be able to easily set up environments with different (dated) versions of WP to test.

This change allows me to provide WP version in the 10updocker create inquiry.
But still defaults to the latest as before.

Alternate Designs

As mentioned by @lippoliv in the issue #157, we could add it via CLI parameter.

Also, we may consider adding the validation of the provided version. But I wanted to keep this PR as small as possible.

Benefits

Introduces a feature to install the desired version of WP, without a need to manually downgrade it afterward. Which reduces the traffic, resource consumption, and chance of an error.

Possible Drawbacks

The inquiry may be too long and verbose.

Verification Process

  • How did you verify that all new functionality works as expected?
  • How did you verify that all changed functionality works as expected?

I run 10updocker create and answered

? Do you want to install WordPress? Yes
? Select a WordPress installation type: Single Site

Then got the new question. Tried default latest, 5.7.3, 5.7, foo.
Then checked what versions of WP were actually installed.

  • How did you verify that the change has not introduced any regressions?

I run the 10updocker create for wordpress.type = dev type, for wordpress = false, and for wordpress.version = latest .

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

Added WordPress version question to create inquiry.

@tomalec
Copy link
Contributor Author

tomalec commented Sep 27, 2021

@eugene-manuilov would you mind taking a look and reviewing this one? :)

@tomalec
Copy link
Contributor Author

tomalec commented Nov 18, 2021

@tylercherpak maybe you may find some time to review this PR?

@tylercherpak tylercherpak self-requested a review January 5, 2022 16:12
@jeffpaul jeffpaul added this to the Future Release milestone Jan 19, 2022
@TylerB24890
Copy link
Contributor

@tylercherpak I went through this one and tested locally -- all worked as expected, but perhaps better error handling of an invalid version would be a good idea.

Right now a version input like foo kills the create command with an error message, but the docker container is already created at this step without a WP install. I think if we could add a fallback to just latest if the version isn't found or even prompt them for a valid version string again, that would be a better experience than failing to install WP at all.

@tomalec
Copy link
Contributor Author

tomalec commented Mar 24, 2022

Thanks, @TylerB24890 for the idea!

image

I implemented the fallback. I agree that prompting again could be a slightly better experience, but given the installation happens far after inquiry, bringing the inquirer back, could mess a bit with the flow. So I went the easier way.

@tomalec
Copy link
Contributor Author

tomalec commented Mar 24, 2022

I was also thinking about making makeInstallWordPress update the settings with the eventually downloaded/tried value in https://github.com/tomalec/wp-local-docker-v2/blob/fix/157-define-WP-version/src/commands/create/install-wordpress.js#L150:L150, but I'm now sure if that's ok to modify the given settings object. How it is used later, do we assume its immutability?

@Remzi1993
Copy link

@TylerB24890 @tomalec This is certainly a nice feature! I really hope this gets merged. It seems stable and workable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define WordPress Version
4 participants