Skip to content
This repository was archived by the owner on Apr 13, 2022. It is now read-only.

Update parity-codec version...#22

Merged
bkchr merged 4 commits intomasterfrom
stas-codec-4-0
Jun 25, 2019
Merged

Update parity-codec version...#22
bkchr merged 4 commits intomasterfrom
stas-codec-4-0

Conversation

@stanislav-tkach
Copy link
Copy Markdown
Contributor

@stanislav-tkach stanislav-tkach commented Jun 13, 2019 •
edited
Loading

...and add some ugly as u64 casting.

Needed for the paritytech/substrate#2855.

@parity-cla-bot
Copy link
Copy Markdown

parity-cla-bot commented Jun 13, 2019

It looks like @DarkEld3r signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

1 similar comment
@parity-cla-bot
Copy link
Copy Markdown

parity-cla-bot commented Jun 13, 2019

It looks like @DarkEld3r signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@stanislav-tkach stanislav-tkach mentioned this pull request Jun 13, 2019
Merged
gui1117
gui1117 reviewed Jun 14, 2019
View reviewed changes
Copy link
Copy Markdown
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.

I'm not sure about whether u32 or u64 is preferred but otherwise good to me

@stanislav-tkach stanislav-tkach requested a review from bkchr June 14, 2019 13:08
bkchr
bkchr reviewed Jun 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Why do you replaced usize with u32 and not u64?

@stanislav-tkach
Copy link
Copy Markdown
Contributor Author

stanislav-tkach commented Jun 17, 2019

@bkchr Honestly, I don't have a strong opinion here. My first thought was to use u64 because it is "closer" to usize. On the other hand, u32 is probably enough.

Should I revert these changes?

@stanislav-tkach
Copy link
Copy Markdown
Contributor Author

stanislav-tkach commented Jun 24, 2019

parity-codec version has been updated to 4.1.
cc @kianenigma

bkchr
bkchr suggested changes Jun 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

If you change all * as u64 into u64::from, we can merge this.

@stanislav-tkach
Copy link
Copy Markdown
Contributor Author

stanislav-tkach commented Jun 24, 2019

@bkchr Unfortunately it cannot be done:

the trait std::convert::From<usize> is not implemented for u64

@bkchr bkchr merged commit 8ac37a1 into master Jun 25, 2019
@bkchr bkchr deleted the stas-codec-4-0 branch June 25, 2019 11:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rphmeier rphmeier Awaiting requested review from rphmeier

@gnunicorn gnunicorn Awaiting requested review from gnunicorn

2 more reviewers

@gui1117 gui1117 gui1117 left review comments

@bkchr bkchr bkchr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-pleasereview

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@stanislav-tkach @parity-cla-bot @bkchr @gui1117