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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow listenHTTP to accept a convenience setting string #1816

Merged
merged 1 commit into from Jul 5, 2017

Conversation

Projects
None yet
3 participants
@wilzbach
Contributor

wilzbach commented Jul 4, 2017

Follow-up to #1810

Now that HTTPServerSettings accept a string, listenHTTP can do so as well 馃帀

Instead of adding five new overloads, I opted for templates.

shared static this()
{
listenHTTP(":11721", (scope req, scope res) {

This comment has been minimized.

@wilzbach

wilzbach Jul 4, 2017

Contributor

I copied the hard-coded port from other tests - how about adding something like getFreePort to Vibe.d?

@wilzbach

wilzbach Jul 4, 2017

Contributor

I copied the hard-coded port from other tests - how about adding something like getFreePort to Vibe.d?

This comment has been minimized.

@s-ludwig

s-ludwig Jul 5, 2017

Member

Or supporting ":0" and providing a way to retrieve the actual listen port.

@s-ludwig

s-ludwig Jul 5, 2017

Member

Or supporting ":0" and providing a way to retrieve the actual listen port.

@s-ludwig

I originally decided against adding this because of the already large number of overloads, but didn't think about converting them to templates to hide that. Should be okay like this.

@s-ludwig s-ludwig added the auto-merge label Jul 5, 2017

@wilzbach

This comment has been minimized.

Show comment
Hide comment
@wilzbach

wilzbach Jul 5, 2017

Contributor

s-ludwig added the auto-merge label 3 hours ago

@s-ludwig are you sure that you configured the webhook correctly here? I can't find any events in the log.
The config should look roughly like this:

image

Contributor

wilzbach commented Jul 5, 2017

s-ludwig added the auto-merge label 3 hours ago

@s-ludwig are you sure that you configured the webhook correctly here? I can't find any events in the log.
The config should look roughly like this:

image

@s-ludwig s-ludwig added auto-merge and removed auto-merge labels Jul 5, 2017

@dlang-bot dlang-bot merged commit fc97b74 into vibe-d:master Jul 5, 2017

4 checks passed

codecov/patch Coverage not affected when comparing 64a63e2...3b5c325
Details
codecov/project 37.025% (+0.008%) compared to 64a63e2
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 5, 2017

Member

Yay, finally worked!

Member

s-ludwig commented Jul 5, 2017

Yay, finally worked!

@wilzbach wilzbach deleted the wilzbach:convenient-listen branch Jul 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment