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

[kvdb-memorydb] Migrated code to 2018 edition, updated parking_lot #222

Merged
merged 2 commits into from
Sep 23, 2019
Merged

[kvdb-memorydb] Migrated code to 2018 edition, updated parking_lot #222

merged 2 commits into from
Sep 23, 2019

Conversation

expenses
Copy link
Contributor

See #143.

@parity-cla-bot
Copy link

It looks like @expenses signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm but I'm a bit worried about the parking_lot update.


[dependencies]
parking_lot = "0.6"
parking_lot = "0.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

We're on 0.8 in parity-ethereum and I have a hunch that this could break things. If you feel like trying this change against the master branch over there, that'd be fantastic.

@expenses
Copy link
Contributor Author

There are some breakages such as

error[E0277]: the trait bound `kvdb_memorydb::InMemory: kvdb::KeyValueDB` is not satisfied
   --> miner/local-store/src/lib.rs:219:30
    |
219 |             let store = super::create(db.clone(), None, Dummy(vec![]));
    |                                       ^^^^^^^^^^ the trait `kvdb::KeyValueDB` is not implemented for `kvdb_memorydb::InMemory`
    |
    = note: required for the cast to the object type `dyn kvdb::KeyValueDB`

Perhaps this should be version 0.2.0?

@dvdplm
Copy link
Contributor

dvdplm commented Sep 23, 2019

@expenses you mean change this crate to v0.2.0? If we upgrade parking_lot to a breaking version then yes.
I haven’t looked closely at the error you posted yet but it could be we need to update all of parity-ethereum at once, or is that what you did perhaps?

The best way imo would be to revert the parking_lot upgrade and get this merged, then we deal with upgrading the reps separately.

And thanks again for your work here! :)

@expenses
Copy link
Contributor Author

Sure, I'll do that.

@dvdplm dvdplm merged commit 085d18b into paritytech:master Sep 23, 2019
ordian added a commit that referenced this pull request Sep 24, 2019
* master:
  [kvdb-memorydb] Migrated code to 2018 edition, updated parking_lot (#222)
  Migrated code to 2018 edition, updated dependencies (#221)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants