Skip to content
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

Add support for custom site domains to valet php and valet composer #1274

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aj-adl
Copy link

@aj-adl aj-adl commented Aug 2, 2022

Fixes #1272

Adds support for passing in a custom sitename via the flag --site=sitename

  • Could be more flexible, at this point the '=' is mandatory
  • Command help updated, arguments added etc
  • Code is repeated in two place, I thought it more likely that potential flags / checks diverge over time and didn't want to prematurely abstract

Potential issues

  • someone calling a php command that requires a "--site" flag has to encapsulate the whole command as a string to pass it in - e.g valet php 'cms_cli do:thing --site=radsite.test' --site=radsite.test

Other considerations

  • It would be better if the isolate function wasn't so tied to dir names - keeping a config file of isolated sites vs just parsing nginx configs seems better to me in the long run but is a larger and more serious refactor of valet.

Fixes laravel#1272

Adds support for passing in a custom sitename via the flag `--site=sitename`
- Could be more flexible, at this point the '=' is mandatory
- Command help updated, arguments added etc
- Code is repeated in two place, I thought it more likely that potential flags / checks diverge over time and didn't want to prematurely abstract

Potential issues
- someone calling a php command that requires a "--site" flag is not going to be able to pass it in.

Other considerations
- It would be better if the isolate function wasn't so tied to dir names - keeping a config file of isolated sites vs just parsing nginx configs seems better to me in the long run but is a larger and more serious refactor of valet.
@mattstauffer
Copy link
Member

@aj-adl It's been a while since I looked at the isolation code. Could you give me an example of a configuration in which this would be necessary?

Thanks!

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

Successfully merging this pull request may close these issues.

Valet isolate proxy CLI commands do not respect custom site names
2 participants