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.

Sign up for GitHub

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

Jump to bottom

fix: update types for Dock #600

Merged
merged 2 commits into from
Jul 9, 2021
Merged

fix: update types for Dock #600

merged 2 commits into from
Jul 9, 2021

Conversation

lovesh
Copy link
Contributor

@lovesh lovesh commented Jul 8, 2021

Signed-off-by: lovesh lovesh.bond@gmail.com

@lovesh
Update types for Dock
20ce605 
Signed-off-by: lovesh <lovesh.bond@gmail.com>
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Jul 8, 2021

User @lovesh, please sign the CLA here.

@TarikGul TarikGul requested review from dvdplm and emostov and removed request for dvdplm July 8, 2021 13:37
TarikGul
TarikGul approved these changes Jul 8, 2021
View reviewed changes
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

LGTM, lastly just run the build.sh file inside of the calc folder in order to update the wasm pkg with your latest changes to the multiplier.

@lovesh
Build wasm package
d50c883 
Signed-off-by: lovesh <lovesh.bond@gmail.com>
@lovesh
Copy link
Contributor Author

lovesh commented Jul 8, 2021

LGTM, lastly just run the build.sh file inside of the calc folder in order to update the wasm pkg with your latest changes to the multiplier.

Done

dvdplm
dvdplm approved these changes Jul 9, 2021
View reviewed changes
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.

Looks sane to me. Only question is if dock-testnet is definitely gone forever and nobody has any interest in its historical data ever again?

calc/src/calc_fee.rs
@@ -50,7 +50,8 @@ impl Multiplier {
("westend", v) if 31 <= v => V2(new_u128(inner)),

("dock-main-runtime", _) => V2(new_u128(inner)),
("dock-testnet", _) => V2(new_u128(inner)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this chain gone from the world forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@emostov
Copy link
Contributor

emostov commented Jul 9, 2021

@TarikGul after this is merged we just need to make sure we publish the calc package. I think its a bit manual with increment version in Cargo.toml, commit/build, and then publish. Also a quick sanity check with the package linked locally if we haven't done that yet

@TarikGul
Copy link
Member

TarikGul commented Jul 9, 2021

@emostov Thanks for the reminder, yea you read my mind. cc: @jsdw I believe you are releasing sidecar next week so this is a good note. I'll get a PR to update the docs for the release notes later today. I need to go through the steps myself again, and do a quick dry run to confirm the process as its been a while.

@TarikGul TarikGul changed the title Update types for Dock fix: update types for Dock Jul 9, 2021
@TarikGul TarikGul merged commit 0300941 into paritytech:master Jul 9, 2021
@lovesh
Copy link
Contributor Author

lovesh commented Jul 10, 2021

Thanks for merging.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dvdplm dvdplm dvdplm approved these changes

@TarikGul TarikGul TarikGul approved these changes

@emostov emostov Awaiting requested review from emostov

@insipx insipx Awaiting requested review from insipx

@maciejhirsz maciejhirsz Awaiting requested review from maciejhirsz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lovesh @emostov @TarikGul @dvdplm