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

Refactoring/cache 6693 #6772

Merged
merged 4 commits into from
Oct 16, 2017
Merged

Refactoring/cache 6693 #6772

merged 4 commits into from
Oct 16, 2017

Conversation

0x7CFE
Copy link
Contributor

@0x7CFE 0x7CFE commented Oct 15, 2017

Affects #6693.

MemoryLruCache was extracted from util/src into separate crate util/memory_cache. Also, references in ethcore was altered to reflect this change.

@0x7CFE 0x7CFE added the A0-pleasereview 🤓 Pull request needs code review. label Oct 15, 2017
@0x7CFE 0x7CFE requested a review from debris October 15, 2017 11:50
@parity-cla-bot
Copy link

It looks like @0x7CFE hasn'signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, plesae reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@0x7CFE
Copy link
Contributor Author

0x7CFE commented Oct 15, 2017

Note: didn't checked for now redundant references to util in ethcore yet.

@0x7CFE
Copy link
Contributor Author

0x7CFE commented Oct 15, 2017

[clabot:check]

@parity-cla-bot
Copy link

It looks like @0x7CFE hasn'signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, plesae reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@0x7CFE
Copy link
Contributor Author

0x7CFE commented Oct 15, 2017

Gosh, CLA bot is stubborn :D

debris
debris reviewed Oct 15, 2017
View reviewed changes
Copy link
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

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

lgtm, there are just two more lines which need to be removed:

util/Cargo.toml:27:lru-cache = "0.1.0"
util/src/lib.rs:103:extern crate lru_cache;

@debris debris added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. M4-core ⛓ Core client code / Rust. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 15, 2017
rphmeier
rphmeier reviewed Oct 15, 2017
View reviewed changes
ethcore/Cargo.toml Outdated
@@ -32,6 +32,7 @@ ethcore-logger = { path = "../logger" }
ethcore-stratum = { path = "../stratum" }
ethcore-util = { path = "../util" }
ethcore-bigint = { path = "../util/bigint" }
memory_cache = { path = "../util/memory_cache" }
Copy link
Contributor

Choose a reason for hiding this comment

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

see above lines: naming convention for crates is with dashes, not hyphens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually, we are very inconstant with it. e.g. rlp_derive, semantic_version, rpc_cli, secret_store and so on...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@0x7CFE
Copy link
Contributor Author

0x7CFE commented Oct 15, 2017

Removed dependencies to lru_cache in .toml

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Oct 15, 2017
@5chdn 5chdn added this to the 1.9 milestone Oct 16, 2017
@debris debris merged commit d1c9acf into master Oct 16, 2017
@debris debris deleted the refactoring/cache-6693 branch October 16, 2017 07:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@debris debris debris left review comments

@rphmeier rphmeier rphmeier left review comments

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
1.9 Velocity
Development

Successfully merging this pull request may close these issues.
5 participants
@0x7CFE @parity-cla-bot @debris @rphmeier @5chdn