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

Addresses #332: pass config-file path for roadrunner #335

Merged
merged 4 commits into from
Jul 5, 2021

Conversation

kevincobain2000
Copy link
Contributor

Submitted with intentions for a first review and consideration.

Reasons for this pull request:

  1. As mentioned in Change path to config file #332: to be able use different .rr.yaml based on different environments
  2. We use deployer. As this is symbolic link deployment, the base_path() picks up the path from the releases/<id> reference instead of the symb link. It'd be nice if we could provide a path via cli.

The first option is more encouraging, as this config file is most likely to be on the server in a different location.

touch(base_path('.rr.yaml'));
return base_path('.rr.yaml');
}
return $this->option('config-path');
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense here to allow different styles of config paths to be passed?

php artisan octane:start --config-path=rr.prod.yml
php artisan octane:start --config-path=config/rr.prod.yml
php artisan octane:start --config-path=./config/rr.prod.yml
php artisan octane:start --config-path=../config/rr.prod.yml
php artisan octane:start --config-path=/etc/roadrunner/rr.prod.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I have added absolute path 58f1dcf

@taylorotwell taylorotwell merged commit a4e9602 into laravel:1.x Jul 5, 2021
@driesvints
Copy link
Member

The flag was renamed to --rr-config.

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.

4 participants