-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
wp-env: Add install-path command. #35638
Conversation
Size Change: 0 B Total Size: 1.13 MB ℹ️ View Unchanged
|
This tested well for me @ribaricplusplus. It would be good to get an entry added to the Readme as part of this PR, eg. after the |
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me! Just have one thought for an addition.
* Logs the path to where wp-env files are installed. | ||
*/ | ||
module.exports = async function installPath() { | ||
const { workDirectoryPath } = await readConfig( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should check if this path exists before outputting it:
~/source/gutenberg
$ ./packages/env/bin/wp-env install-path
/home/nt/.wp-env/2ccfc3290b190367751be3c7bce4c188
~/source/gutenberg wp-env/install-path
$ cd ..
~/source # wp-env is not used here
$ ./gutenberg/packages/env/bin/wp-env install-path
/home/nt/.wp-env/e7bffa65797a36b9089db69d81c0879d
readConfig
(if I recall) just generates a config based on everything available. But nothing exists until wp-env start
has been executed. I think it might be helpful to output an error in the case that the work directory path doesn't exist yet, rather than just showing what it would be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the readme entry, and the additional path check is testing well for me also 🎉
*/ | ||
module.exports = async function installPath() { | ||
const configPath = path.resolve( '.wp-env.json' ); | ||
if ( ! fs.existsSync( configPath ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for going back and forth on this so much! It is possible to run wp-env
without the JSON configuration file (iirc), so I might remove this portion of the check. Otherwise this looks great 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, slipped my mind! You're right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove it today or tomorrow... Or you can do it if you feel ilke it :)
176028f
to
8945640
Compare
Description
I have many
wp-env
instances on my machine for various projects I'm working on. When I go to thewp-env
folder where all these instances are installed, I'm met with a bunch of hashes and I can't figure out which project is installed where.This PR adds an
install-path
command that logs to the console where this particular instance's files are.Screenshots
Types of changes
New feature
Checklist:
*.native.js
files for terms that need renaming or removal).