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

Fix linked map for trait types and Option#1809

Merged
gavofyork merged 11 commits intomasterfrom
td-fixlinked
Feb 18, 2019
Merged

Fix linked map for trait types and Option#1809
gavofyork merged 11 commits intomasterfrom
td-fixlinked

Conversation

@tomusdrw
Copy link
Contributor

@tomusdrw tomusdrw commented Feb 15, 2019

Fixes linked map in cases like linked map: T::Balance => Option<T::Balance>
Some groundwork was done by @thiolliere already in #1800, it also contains a bit more improvements over the impl.

@tomusdrw tomusdrw added A0-please_review Pull request needs code review. M4-core labels Feb 15, 2019
@tomusdrw tomusdrw requested a review from gui1117 February 15, 2019 14:54
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

also maybe it would be good to have a quick test on Option storage also, as logic differ a bit.

mod #internal_module {
use super::*;
let helpers = quote! {
#[derive(parity_codec_derive::Encode, parity_codec_derive::Decode)]
Copy link
Contributor

Choose a reason for hiding this comment

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

diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs
index fbd40318..888cd25a 100644
--- a/srml/example/src/lib.rs
+++ b/srml/example/src/lib.rs
@@ -62,6 +62,7 @@ decl_storage! {
                // equipped with `fn getter_name() -> Type` for basic value items or
                // `fn getter_name(key: KeyType) -> ValueType` for map items.
                Dummy get(dummy) config(): Option<T::Balance>;
+               Dummy2 get(dummy) config(): linked_map T::Balance => Option<T::Balance>;
 
                // this one uses the default, we'll demonstrate the usage of 'mutate' API.
                Foo get(foo) config(): T::Balance;

result in

error[E0433]: failed to resolve: use of undeclared type or module `parity_codec_derive`
  --> srml/example/src/lib.rs:36:1
   |
36 | / decl_storage! {
37 | |     // A macro for the Storage trait, and its implementation, for this module.
38 | |     // This allows for type-safe usage of the Substrate storage database, so you can
39 | |     // keep things around between blocks.
...  |
69 | |     }
70 | | }
   | |_^ use of undeclared type or module `parity_codec_derive`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.
error: Could not compile `srml-example`.

probably #scrate::parity_codec_derive::Encode and same Decode would fix it

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but because of the module created the modified example still fails I don't really know why.

@gui1117
Copy link
Contributor

gui1117 commented Feb 15, 2019

I saw your renaming, I wonder those function should be private isn't it ?
if yes maybe we can do even more be insert underscore in front of it and also making the generated doc /// NOTE: internal use and making the current doc as code doc //
what do you think ?
(If we want user to be able to use it then I'm thinking maybe we should provide them under a trait in support/storage/ or something)

@gavofyork gavofyork removed the A0-please_review Pull request needs code review. label Feb 17, 2019
@tomusdrw
Copy link
Contributor Author

tomusdrw commented Feb 18, 2019

@thiolliere I've added the internal module back and the helper functions are now part of the Utils trait.
The module name clearly indicates that anything from it should only be used internally, I think it will be a clear signal that this stuff is not meant to be used by developers directly.

@tomusdrw tomusdrw added A0-please_review Pull request needs code review. and removed A4-gotissues labels Feb 18, 2019
@gavofyork gavofyork merged commit edf2a8f into master Feb 18, 2019
@gavofyork gavofyork deleted the td-fixlinked branch February 18, 2019 18:23
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
* Fix linked map for traits.

* Fix Option<_> variant.

*  Improve naming a tad

* Rebuild runtime

* Encapsulate private data in the inner module.

* Bump impl version.

* Fix deriving codec in srml-example.

* Fix derivation without importing parity-codec-derive.

* Fix config() for map.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants