-
Notifications
You must be signed in to change notification settings - Fork 157
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
feat(server): return ListeningServer
from Nickel::listen
#334
Conversation
/// use nickel::Nickel; | ||
/// | ||
/// let server = Nickel::new(); | ||
/// server.listen("127.0.0.1:6767"); | ||
/// let listening = server.listen("127.0.0.1:6767").expect("Failed to launch server"); | ||
/// println!("Listening on: {:?}", listening.socket()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this print be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was to show the use of socket()
to get the address, but I guess this example will print twice (since the options were not passed to disable printing). So I think it's fine, but I should make the intention more explicit perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I see. As you wish, I think we can just leave it like this.
Exciting! This looks nice to me, but I'm not known to do a lot of testing so let's see if we can get some user feedback. |
☔ The latest upstream changes (presumably 2d91a0a) made this pull request unmergeable. Please resolve the merge conflicts. |
Rebased on master |
☔ The latest upstream changes (presumably f7658fa) made this pull request unmergeable. Please resolve the merge conflicts. |
Is this still in flight? |
…erver is listening on BREAKING CHANGE: This changes the return type of Nickel::listen so it may break some code. To fix broken code while maintaining the previous semantics, you can add `unwrap` to the `listen` call.
@euclio just rebased. Do you have any suggested edits / concerns? If not I will try to merge this tomorrow. |
Nope, looks good to me. Thanks for your work! On Wed, Jun 22, 2016 at 10:25 PM Ryman [email protected] wrote:
|
…ion tests When this test is included in the integration suite, the import is no longer in the root of the crate so the macro_use attribute is disallowed.
@Ryman friendly ping :) |
@homu r+ |
📌 Commit c9abad7 has been approved by |
…erver is listening on BREAKING CHANGE: This changes the return type of Nickel::listen so it may break some code. To fix broken code while maintaining the previous semantics, you can add `unwrap` to the `listen` call. Pull request: #334 Approved by: Ryman
Pull request: #334 Approved by: Ryman
…tabase Pull request: #334 Approved by: Ryman
…ion tests When this test is included in the integration suite, the import is no longer in the root of the crate so the macro_use attribute is disallowed. Pull request: #334 Approved by: Ryman
☀️ Test successful - status |
…erver is listening on BREAKING CHANGE: This changes the return type of Nickel::listen so it may break some code. To fix broken code while maintaining the previous semantics, you can add `unwrap` to the `listen` call. Pull request: nickel-org#334 Approved by: Ryman
Pull request: nickel-org#334 Approved by: Ryman
…tabase Pull request: nickel-org#334 Approved by: Ryman
…ion tests When this test is included in the integration suite, the import is no longer in the root of the crate so the macro_use attribute is disallowed. Pull request: nickel-org#334 Approved by: Ryman
Related to #328.
The tests within this repo are a bit of a hack (require shelling out to subprocesses involving cargo), with this change we can instead make it easier to integration test a server (or multiple instances of one) all within the same process which should be easier and faster.
The code within the integration test example would get tested using a normal
cargo test
, but since it's in our example files I had to use a workaround to get it compiled directly into our integration test runner.BREAKING CHANGE: This changes the return type of
Nickel::listen
, you can add anunwrap()
to the call to maintain previous semantics.