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

[1.x] Read port for http server from environment if no port is passed #605

Merged
merged 4 commits into from
Oct 31, 2022
Merged

[1.x] Read port for http server from environment if no port is passed #605

merged 4 commits into from
Oct 31, 2022

Conversation

hendrikheil
Copy link
Contributor

This pull request aims to add support for defining the HTTP server port through environment variables. The implementation shouldn't break any existing behavior.
This feature should be helpful for anyone that needs to run octane with a randomly generated port.

@hendrikheil hendrikheil marked this pull request as ready for review October 31, 2022 09:57
@driesvints driesvints requested a review from nunomaduro October 31, 2022 10:34
@taylorotwell taylorotwell merged commit 40327ec into laravel:1.x Oct 31, 2022
@@ -16,7 +16,7 @@ class StartCommand extends Command implements SignalableCommandInterface
public $signature = 'octane:start
{--server= : The server that should be used to serve the application}
{--host=127.0.0.1 : The IP address the server should bind to}
{--port=8000 : The port the server should be available on}
Copy link
Contributor

Choose a reason for hiding this comment

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

@hendrikheil This change is safe but slightly inconvenient as it removes the default port from the --help text of the command:

// Before
--port[=PORT]                  The port the server should be available on [default: "8000"]

// After
--port[=PORT]                  The port the server should be available on

Might be worth to add this to the text 👍

Copy link
Member

Choose a reason for hiding this comment

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

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