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

Ethernet feature gate #308

Closed
wants to merge 1 commit into from

Conversation

rubdos
Copy link
Contributor

@rubdos rubdos commented Oct 4, 2019

This should be - as far as I could see - a feature-gate for the Ethernet code.

In any case, I like the idea of adding a feature for turning off EthernetInterface far more than splitting into many crates.

(cited @whitequark from this comment)

If you see anything I missed that should be behind this feature gate, let me know. I enable Ethernet by default, as to stay backwards compatible.

I still have a bunch of warnings when compiling without ethernet, which is why I have 8c441c3. If you see how to fix that, always welcome!

I hope this is useful for anyone else :-)

Copy link
Contributor

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

LGTM, please squash the commits and I'll merge

Disable ethernet-related features and framing based on a feature gate.

Add a no-ethernet option to Travis.

notes:
- allow(unused) is added when not using ethernet feature
- Don't run pretty-print doctest, ethernet is optional.
@rubdos
Copy link
Contributor Author

rubdos commented Oct 4, 2019

Done, and CI approved :-)

@whitequark
Copy link
Contributor

@m-labs-homu r+

@m-labs-homu
Copy link

📌 Commit 8feda11 has been approved by whitequark

@m-labs-homu
Copy link

⌛ Testing commit 8feda11 with merge 14bad97...

@m-labs-homu
Copy link

☀️ Test successful - status-travis
Approved by: whitequark
Pushing 14bad97 to master...

@rubdos rubdos deleted the ethernet-feature-gate branch October 7, 2019 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants