Skip to content

Proposals update. Parameters migration from the Storage to the Trait.#1680

Merged
mnaamani merged 27 commits intoJoystream:proposals_updatefrom
shamil-gadelshin:proposals_update6
Nov 18, 2020
Merged

Proposals update. Parameters migration from the Storage to the Trait.#1680
mnaamani merged 27 commits intoJoystream:proposals_updatefrom
shamil-gadelshin:proposals_update6

Conversation

@shamil-gadelshin
Copy link
Copy Markdown
Contributor

Summary

We created a separate runtime module(file) In order to be able to pass the proposal parameter constants during the compilation time. The primary goal for the conditional compilation is integration tests. All parameters for the proposal could be set via ALL_PROPOSALS_PARAMETERS_JSON environment variable. The value of the variable should be a valid JSON file (a sample file is included in the PR). The PR contains a migration of the single proposal parameter set for demo purposes.

Comments

  • We used lite-json crate as "no_std" option for parsing JSON recommended for Substrate. serde_json is not 'no_std' ready. serder_json_core library is not working.
  • Tests are ready for the code but disabled (marked ignore).
  • Corrupted JSON resulted in panic
  • JSON with incorrect types (eg.: string instead of integer) resulted in panic
  • 0 (zero) value for the optional type becomes None.
  • Non-existent fields and parameter groups are set with default values.

The previous draft PR could be found here.

# Conflicts:
#	runtime/src/proposals_configuration/mod.rs
@mnaamani
Copy link
Copy Markdown
Member

Only issue here is that the CI checks specifically for network integration tests are from an old branch.
After merging we should probably update proposals_update branch from babylon.

# Conflicts:
#	Cargo.lock
#	runtime-modules/proposals/discussion/Cargo.toml
@mnaamani mnaamani merged commit 00cf4d4 into Joystream:proposals_update Nov 18, 2020
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.

2 participants