-
Notifications
You must be signed in to change notification settings - Fork 4.6k
wp-env: Add install-path command. #35638
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
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 |
noahtallen
left a comment
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/e7bffa65797a36b9089db69d81c0879dreadConfig (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.
glendaviesnz
left a comment
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-envinstances on my machine for various projects I'm working on. When I go to thewp-envfolder 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-pathcommand that logs to the console where this particular instance's files are.Screenshots
Types of changes
New feature
Checklist:
*.native.jsfiles for terms that need renaming or removal).