Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Optional ethcore features should be enabled by default #9103

Closed
tomusdrw opened this issue Jul 12, 2018 · 1 comment
Closed

Optional ethcore features should be enabled by default #9103

tomusdrw opened this issue Jul 12, 2018 · 1 comment
Labels
B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@tomusdrw
Copy link
Collaborator

Recently some parts for ethcore and ethcore-miner were made optional: see #9020

However, we import ethcore and ethcore-miner crate from multiple places, an all of those places should specify the required features. It is however easly forgotten.

We should make the features default, and other crates that don't require them should just include the crate with no-default-features.

Also using the features if they are disabled should trigger an error. Currently 2.0 and 2.1 just silently fail to work correctly. Seems that work-notify, price-info and stratum are just broken currently.

@tomusdrw tomusdrw added F2-bug 🐞 The client fails to follow expected behavior. P5-sometimesoon 🌲 Issue is worth doing soon. M4-core ⛓ Core client code / Rust. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. and removed B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. labels Jul 12, 2018
@tomusdrw
Copy link
Collaborator Author

My bad, it seems that the features are set up correctly, although work-notify is the only one broken, see:
https://github.com/paritytech/parity/pull/9020/files#r201957185

That said I still believe that the features should be on by default. It never makes sense to compile Parity without them, it should only be used as an optimization feature for crates dependent on ethcore.

@5chdn 5chdn added this to the 2.0 milestone Jul 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

2 participants