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

Add type annotation pattern #6

Merged
merged 1 commit into from
Dec 15, 2018
Merged

Conversation

tathanhdinh
Copy link
Contributor

Simple change to handle #5

Copy link
Owner

@lambda-fairy lambda-fairy left a comment

Choose a reason for hiding this comment

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

Looks good so far.

Can you please add an example to the documentation as well?

src/lib.rs Outdated Show resolved Hide resolved
@lambda-fairy
Copy link
Owner

Also, the Travis build fails on Rust 1.7.0. Can you please investigate?

I'm happy to bump the minimum supported version, but I'd like to know the reason behind the error first.

Thanks!

@tathanhdinh
Copy link
Contributor Author

tathanhdinh commented Oct 18, 2018

@lfairy sorry for the delay, I've updated the PR.
I've looked into the problem of failed compilation with rustc-1.7.0, unfortunately I still don't know what exactly the reason is. Maybe it comes from a bug of rust's parser at this version, there was a similar issue: rust-lang/rust#18775 (comment)

@lambda-fairy
Copy link
Owner

Thanks for looking into it.

I played around with the build and found that Rust 1.11.0 works but 1.10.0 doesn't. I'm happy with bumping the version requirement to 1.11.0.

Maybe it comes from a bug of rust's parser at this version, there was a similar issue: rust-lang/rust#18775 (comment)

That issue probably isn't relevant because it was fixed in 2014 when 1.11.0 was released in 2016.

Looking at the release notes, rust-lang/rust#34436 feels more likely. I found that nightly-2016-06-24 fails but nightly-2016-07-02 passes, which is strong evidence for that PR being the fix.

Can you please bump the minimum version to 1.11.0 and add an example to the documentation? Once that's done then I'll be happy to merge.

@hcpl
Copy link
Contributor

hcpl commented Dec 1, 2018

@tathanhdinh are you still working on this PR? I would like to get this merged because I have a PR idea that touches your code along with other parts of if_chain!.

@tathanhdinh
Copy link
Contributor Author

tathanhdinh commented Dec 2, 2018

@hcpl

@tathanhdinh are you still working on this PR? I would like to get this merged because I have a PR idea that touches your code along with other parts of if_chain!.

Sorry, I forgot it. I will push the changed PR today.

@lambda-fairy lambda-fairy merged commit 66cbe94 into lambda-fairy:master Dec 15, 2018
@lambda-fairy
Copy link
Owner

Sorry about the delay! Looks good to me.

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