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

Jump to bottom

Upgrade hyper/mime/url #319

Closed
wants to merge 2 commits into from
Closed

Upgrade hyper/mime/url #319

wants to merge 2 commits into from

Conversation

abonander
Copy link
Contributor

The examples/template.rs example fails but it says it's deprecated? Anyways, the assertion fails because the line endings changed, probably because I'm on Windows and Git checks out with CRLF line endings by default.

failures:

---- examples::template::renders_data stdout ----
        Parsed: port=58433 from "Listening on http://[::1]:58433"
thread 'examples::template::renders_data' panicked at 'assertion failed: `(left == right)` (left: `"<html>\r\n
   <head>\r\n        <title>\r\n            nickel.rs - example\r\n        </title>\r\n    </head>\r\n    <body>\r\n    <h1>\r\n        Hello user!\r\n    </h1>\r\n    </body>\r\n</html>"`, right: `"<html>\n    <head>\n
     <title>\n            nickel.rs - example\n        </title>\n    </head>\n    <body>\n    <h1>\n        Hello user!\n    </h1>\n    </body>\n</html>"`)', tests\examples\template.rs:23
Unparsed Stdout:

Edit: forgot to add this:

Closes #318

@GitCop
Copy link

GitCop commented Mar 22, 2016

Thanks for contributing! Unfortunately, I'm here to tell you there were style issues with your Pull Request:

  • Commit: d1168a9
    • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/nickel-org/nickel.rs/blob/master/contributing.md

This message was auto-generated by https://gitcop.com

@abonander
Copy link
Contributor Author

I can fix the commit message but the link to the conventions document from CONTRIBUTING.md is broken, so I'm not sure what to put there.

Ryman
Ryman reviewed Mar 22, 2016
View reviewed changes
Cargo.toml
@@ -1,7 +1,7 @@
[package]

name = "nickel"
version = "0.7.3"
version = "0.8.0"
Copy link
Member

Choose a reason for hiding this comment

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

Could you leave the version bump out of this commit? I'd prefer if we bumped it alongside a changelog update.

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

@cybergeek94 Ah, thanks for the heads up, I'll fix the link, it looks like they've moved repos to here.

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

Looks like travis is failing and we won't be able to support 1.3 anymore since hyper's now forcing the timeouts feature, and one of our dependencies is using Result::expect. So it seems 1.4 is the new minimum.

@abonander
Copy link
Contributor Author

What would be the type and scope of this commit? I guess it would be a breaking change as well.

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

feat(*): bump hyper, mime and url dependency versions would be fine, and you're right it is a breaking change :)

@abonander
feat(*): bump hyper, mime and url dependency versions
7e4f492 
BREAKING CHANGE: More recent versions of the `hyper`, `mime` and `url` dependencies
are now required.
@abonander
Copy link
Contributor Author

@Ryman amended, force-pushed.

@abonander
Copy link
Contributor Author

Ah, still needs to upgrade to a minimum of Rust 1.4 as you said. Do you want me to do that here? In a new commit or amend the current one?

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

@cybergeek94 A new commit sounds fine, can add a basic description of why in the description 👍

@abonander
Copy link
Contributor Author

@Ryman how's that look?

@abonander
Copy link
Contributor Author

Crap, looks like we might need to go even newer than that. The iter_order feature was stabilized in 1.5. Is that too new for a minimum version?

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

Ah, that's a shame but I'm happy enough to move to it if that's what significant dependencies (url) have moved to. cc @cburgdorf @simonpersson

@Ryman
Copy link
Member

Ryman commented Mar 22, 2016

Might want to adjust the message as it's not really hyper's fault, more just the general ecosystem has moved on :P

@abonander
feat(Rust): Upgrade minimum Rust version to 1.5
f4cc740 
Required for upgraded dependencies to build on stable Rust.

BREAKING CHANGE: The minimum Rust version to build Nickel is now 1.5.
@abonander
Copy link
Contributor Author

Okay, let's see if that works.

@abonander
Copy link
Contributor Author

Passed this time, though there's some failed tests on nightly (which Travis allows, so not sure that's a problem?)

@Ryman
Copy link
Member

Ryman commented Apr 2, 2016

Merged as part of #321, thanks for your contribution!

@Ryman Ryman closed this Apr 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@abonander @GitCop @Ryman