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

Allow consumer to configure styx test api server ports. #187

Merged
merged 6 commits into from
Jun 13, 2018

Conversation

mikkokar
Copy link
Contributor

@mikkokar mikkokar commented Jun 13, 2018

Makes it possible for plugin tests to configure HTTP port numbers for the test server.

@mikkokar mikkokar requested review from kvosper and dvlato June 13, 2018 14:56
private int proxyHttpsPort;

/**
* Specifies the HTTP port for proxy server.
Copy link
Contributor

Choose a reason for hiding this comment

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

These methods should have javadoc comments explain that the endpoints will be assigned random free ports if not set explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Side note: Speaking of documentation, we do not have actual documentation for using the test-api yet, but we have an issue #177 regarding this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will fix the documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Setting it to zero will do it, but it's probably a more likely use case for someone to simply not call the method at all. I would phrase the comment like:
By default, Styx will automatically allocate a free port number. This happens when a port is not set a value on the builder, or it is set a value of 0 (zero).

private int proxyHttpsPort;

/**
* Specifies the HTTP port for proxy server.
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting it to zero will do it, but it's probably a more likely use case for someone to simply not call the method at all. I would phrase the comment like:
By default, Styx will automatically allocate a free port number. This happens when a port is not set a value on the builder, or it is set a value of 0 (zero).

@mikkokar mikkokar merged commit fc6892b into ExpediaGroup:styx-1.0-dev Jun 13, 2018
@mikkokar mikkokar deleted the test-api-server-ports branch June 13, 2018 16:54
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.

3 participants