search

pandeiro / boot-http

A simple HTTP serve task for the Boot build tool
62 stars 34 forks source link

Feature/make port available to other tasks #36

Closed codebeige closed 8 years ago

codebeige commented 8 years ago

This will make the actual port being used available to other tasks through :http-port on the fileset. That way it is now possible to pass an option of --port 0 and make the task autodetect a random free port. See https://github.com/pandeiro/boot-http/issues/34 for a preceding discussion.

I also did some refactoring in order to pass the information from the http/server call in a clean way and get rid of httpkit branching all over the place.

martinklepsch commented 8 years ago

Great! I think I'd suggest to remove the part of adding the port to the fileset for now. IMHO that's a separate concern from allowing users to randomly choose a port and report it properly.

codebeige commented 8 years ago

I was mainly interested in making the port available to subsequent tasks as well. When starting up a server for acceptance or integration testing, I want to bind it to any free port and then run the tests in a browserish environment (like PhantomJS). This is especially true for CI environments or multiple test runners. But the test tasks need to know about the port here. I thought adding meta data to the fileset was the right way to go. Do you disagree?

martinklepsch commented 8 years ago

I see. I'm still thinking this would be better handled as two separate PRs that build on each other.

In any case I think adding keys to the fileset record is an unusual way of adding metadata. Usually metadata is added to separate TmpFile records. My initial intuition would be to put a file containing the port into the fileset but maybe yours is also fine or there are other better approaches, to be safe I'd suggest asking for feedback on that one particular line in the #boot channel :)

codebeige commented 8 years ago

I was also thinking of a .http-port file or similar first. But then I read this discussion about meta data on boot tasks: http://hoplon.discoursehosting.net/t/best-practices-for-creating-extensible-tasks/358/5

Especially the following quote:

It's also possible to assoc directly onto the fileset object itself for project-level metadata, if desired.

I can still split the PR, of course. Because of the refactoring the two PRs will not really end up to be independent, though.

martinklepsch commented 8 years ago

Maybe then after all it is the right way :)

two PRs will not really end up to be independent, though.

Sure, I think that's fine, we just need to merge the refactoring/random port reporting thing first and then the other :)

codebeige commented 8 years ago

I will split them up. Thank you.

pandeiro commented 8 years ago

Thanks @martinklepsch @codebeige for the work and discussion on this.