wp-local-docker-v2 icon indicating copy to clipboard operation
wp-local-docker-v2 copied to clipboard

Provide WP version in the inquiry.

Open tomalec opened this issue 4 years ago • 6 comments

Fixes https://github.com/10up/wp-local-docker-v2/issues/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 https://github.com/10up/wp-local-docker-v2/issues/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:

  • [x] I have read the CONTRIBUTING document.
  • [x] 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.
  • [x] All new and existing tests passed.

Applicable Issues

  • https://github.com/10up/wp-local-docker-v2/issues/157

Changelog Entry

Added WordPress version question to create inquiry.

tomalec avatar Sep 10 '21 16:09 tomalec

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

tomalec avatar Sep 27 '21 10:09 tomalec

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

tomalec avatar Nov 18 '21 21:11 tomalec

@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.

TylerB24890 avatar Feb 02 '22 14:02 TylerB24890

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 avatar Mar 24 '22 11:03 tomalec

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?

tomalec avatar Mar 24 '22 11:03 tomalec

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

Remzi1993 avatar Sep 17 '22 01:09 Remzi1993