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

Decl_storage macro : force traits. #1317

Merged
merged 6 commits into from
Jan 7, 2019
Merged

Decl_storage macro : force traits. #1317

merged 6 commits into from
Jan 7, 2019

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Dec 21, 2018 •
edited
Loading

This PR fixes #1312 by specifying explicitly the trait used for this instruction.
Also fixes #1334 by specifying correctly Vec usage.

@cheme cheme added the A0-please_review Pull request needs code review. label Dec 21, 2018
@bkchr
Copy link
Member

bkchr commented Dec 22, 2018

For #1313 I think it is better to not generate the GenesisConfig, if it is not required at all. I would rather expect the documentation to reflect, which parameter does what.
For example, also map is not supported in the GenesisConfig. The user would still be confused, if a GenesisConfig is generated, but without any fields.

@0x7CFE
Copy link
Contributor

0x7CFE commented Dec 22, 2018 •
edited
Loading

PS : github diff is super hard to read on the second commit (genesis config part), I just did remove a condition and indent -1 the code.

@cheme, you may always use this option to filter such changes:
ws

@cheme cheme changed the title Decl_storage macro : force traits and genesisconfig. Decl_storage macro : force traits. Dec 24, 2018
@cheme
Copy link
Contributor Author

cheme commented Dec 24, 2018

I agree that it is better to avoid genesis config (I did revert the commit).
The issue with #1313 seems to simply be the construct_runtime macro using Config parameter by default (hence querying genesis config). Should we remove Config out of the default value?
I also did observe that GenesisConfig is not available in no_std (not sure why).

@0x7CFE
Copy link
Contributor

0x7CFE commented Dec 24, 2018

I also did observe that GenesisConfig is not available in no_std (not sure why).

Gavin mentioned that it's used only in std environment.

@bkchr
Copy link
Member

bkchr commented Dec 24, 2018

Yeah Genesisconfig is only required on std and is not used in wasm.

@gavofyork gavofyork requested a review from bkchr January 7, 2019 14:58
@bkchr bkchr merged commit 6cef98d into paritytech:master Jan 7, 2019
gguoss pushed a commit to chainpool/substrate that referenced this pull request Jan 11, 2019
@cheme @gguoss
Decl_storage macro : force traits. (paritytech#1317)
500d270 
* Missing trait def on two calls.

Slight mcla refact on transfo.

* Allways provide GenesisConfig even when useless (for runtime module
export).

* Revert "Allways provide GenesisConfig even when useless (for runtime module"

This reverts commit 84a29bc.

* Fix Vec usage (from rstd).
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
@cheme @MTDK1
Decl_storage macro : force traits. (paritytech#1317)
a0b60f3 
* Missing trait def on two calls.

Slight mcla refact on transfo.

* Allways provide GenesisConfig even when useless (for runtime module
export).

* Revert "Allways provide GenesisConfig even when useless (for runtime module"

This reverts commit 84a29bc.

* Fix Vec usage (from rstd).
liuchengxu pushed a commit to liuchengxu/substrate that referenced this pull request May 31, 2023
@nazar-pc
Merge pull request paritytech#1317 from subspace/archived-segment-ref…
85af975 
…actoring

Archived segment refactoring
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gavofyork gavofyork gavofyork approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Milestone
No milestone
4 participants
@cheme @bkchr @0x7CFE @gavofyork