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

Change 2 booleans to 1 enum #103

Merged

Conversation

pwoolcoc
Copy link
Contributor

Since socket.new_pollfd(false, false) is not a particularly useful state
to be in, change this API to use an enum instead of 2 booleans. This allows
us to limit the API to only the states that are actually useful

@blabaere
Copy link
Collaborator

@pwoolcoc The changes look good to me but both Travis and my own machine are telling that the latest rust nightly package have wreaked havoc on cargo. Testing with rustc 1.0.0-nightly (c4fe7d6ae 2015-02-23) (built 2015-02-24). Will check again later.

@thehydroimpulse
Copy link
Owner

I believe the last nightly broke cargo or something according to the PSA on reddit.

@pwoolcoc
Copy link
Contributor Author

Yea, I had to revert to 2015-02-22 to get cargo to work properly

@pwoolcoc
Copy link
Contributor Author

I checked the travis build this morning, and it reminded me that I had forgotten to fix the tests :-/. So, I just pushed a fix for those, along with one more docs fix.

@thehydroimpulse
Copy link
Owner

@pwoolcoc Awesome, thanks a lot!

@blabaere blabaere mentioned this pull request Feb 27, 2015
@blabaere
Copy link
Collaborator

@pwoolcoc PR #103 does not build with the latest rust nightly because it is based on a version of the master that did not build. You should fetch the changes from the master and push them on your repo. This should fix the build.

Since `socket.new_pollfd(false, false)` is not a particularly useful state
to be in, change this API to use an enum instead of 2 booleans. This allows
us to limit the API to only the states that are actually useful
@pwoolcoc
Copy link
Contributor Author

@blabaere all set!

@blabaere
Copy link
Collaborator

@thehydroimpulse I agree with Travis, we can safely merge PR #103. This would make the API more usable.

thehydroimpulse added a commit that referenced this pull request Feb 28, 2015
@thehydroimpulse thehydroimpulse merged commit 42751b3 into thehydroimpulse:master Feb 28, 2015
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.

None yet

3 participants