Skip to content

wp-now : Add/support for wpenv config #263

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 26 commits into from
May 17, 2023
Merged

wp-now : Add/support for wpenv config #263

merged 26 commits into from
May 17, 2023

Conversation

kozer
Copy link
Contributor

@kozer kozer commented May 8, 2023 •
edited
Loading

Related to: #230

This pr supports reading .wp-env.json, and .wp-env.override.json and converting them to wp-env.json format.

Testing Instructions

  • Pull the changes from this branch to your local environment
  • Run nvm use
  • Run yarn install
  • Run yarn build
  • Install nx globally npm install -g nx@latest
  • Ensure that you have a folder with a plugin
  • Run npx nx preview wp-now start --path=<path-to-plugin>
  • Ensure that you see the default options printed out.
  • Add a .wp-env.json file to your plugin directory:
{
	"core": "6.2"
}
  • Run npx nx preview wp-now start --path=<path-to-plugin>
  • Make sure that you see wp: 6.2 in the console
  • Create a .wp-env.override.jsonfile in your plugin directory:
{
	"core": "6.33"
}
  • Ensure that you see wp: 6.33 in the console (You will receive an error but that's ok)

@kozer kozer marked this pull request as draft May 8, 2023 14:32
@sejas
Copy link
Collaborator

sejas commented May 8, 2023

@kozer kozer force-pushed the add/support_wpenv_config branch from ca5c5bb to 7721a1d Compare May 10, 2023 10:01
@kozer kozer marked this pull request as ready for review May 11, 2023 12:11
@kozer kozer requested a review from adamziel May 11, 2023 12:24
@kozer kozer changed the title Add/support wpenv config wp-now : Add/support for wpenv config May 11, 2023
@adamziel
Copy link
Collaborator

I'm about to leave for WC Gliwice and won't have a chance to review in details, but just wanted to thank you for working through my grumpy notes @kozer – this looks super clear now ❤️ Thinking forward, the getWpNowConfig() may eventually cannibalize parseOptions() from #302

@danielbachhuber
Copy link
Member

Let's wait to merge this until #302 and #305 have landed. Once they land, we should make sure this PR is updated accordingly and includes tests.

@kozer
Copy link
Contributor Author

kozer commented May 11, 2023

I'm about to leave for WC Gliwice and won't have a chance to review in details, but just wanted to thank you for working through my grumpy notes @kozer – this looks super clear now heart Thinking forward, the getWpNowConfig() may eventually cannibalize parseOptions() from #302

You are welcome. Actually, this was pretty much my initial thought. And then my OOP self came out. :P
Thanks for your comments!

@kozer
Copy link
Contributor Author

kozer commented May 16, 2023

@danielbachhuber , @sejas, @adamziel , it's updated to the latest trunk.
I have also refactored, and moved some things to config.
Also added some tests.

@kozer kozer requested review from danielbachhuber and sejas May 16, 2023 11:19
@danielbachhuber
Copy link
Member

@kozer Can you resolve the merge conflict again? Also, can you update packages/wp-now/README.md on this branch to include some mention of which wp-env.json arguments are supported?

@adamziel Is this refactor roughly in line with what you were thinking?

@kozer
Copy link
Contributor Author

kozer commented May 17, 2023

@danielbachhuber Updated! Thanks! cc: @adamziel

@danielbachhuber
Copy link
Member

@kozer This still needs to be done too:

Also, can you update packages/wp-now/README.md on this branch to include some mention of which wp-env.json arguments are supported?

adamziel
adamziel approved these changes May 17, 2023
View reviewed changes
Copy link
Collaborator

@adamziel adamziel left a comment

Choose a reason for hiding this comment

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

I left a comment related to code style, but other than that it looks good. Thank you @kozer !

@danielbachhuber danielbachhuber requested a review from wojtekn May 17, 2023 13:25
@danielbachhuber danielbachhuber merged commit a2e1065 into WordPress:trunk May 17, 2023
@sejas sejas deleted the add/support_wpenv_config branch May 17, 2023 14:27
@wojtekn wojtekn mentioned this pull request May 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamziel adamziel

@danielbachhuber danielbachhuber

@sejas sejas

@wojtekn wojtekn

Assignees

@kozer kozer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kozer @sejas @adamziel @danielbachhuber