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.

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

Serialize block count as quantity 2 #669

Merged
merged 7 commits into from
Dec 10, 2021
Merged

Serialize block count as quantity 2 #669

merged 7 commits into from
Dec 10, 2021

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Dec 10, 2021

Sorry, there was an issue with #668

I created the PR too quickly and didn't run cargo check 🙈. It turns out there was an issue where I was moving block_count when doing block_count.into() and needed to first convert block_count to a U256 to avoid move issues.

Sorry for breaking your master builds!

PR Checklist

  • Added Tests (No - but should be covered by existing tests?)
  • Added Documentation (No - behaviour shouldn't have been changed)
  • Updated the changelog (No - just fixes my broken PR).

Also, make sure to run check commands:

$ cargo check --all-features
$ cargo +nightly fmt --all
$ cargo build --all-features
$ cargo test --all-features

@gakonst
Copy link
Owner

gakonst commented Dec 10, 2021

7bd42be I see this is already in master?

@nlordell
Copy link
Contributor Author

nlordell commented Dec 10, 2021

I see this is already in master?

Sorry, I got messed up and was looking at my fork. Anyway, I updated the description.

There is a build issue there where I was moving block_count on line 865 and (when calling block_count.into()) and again on line 871 when retrying (when calling block_count.into().as_u64()) but T: !Copy.

I created the PR too quickly while waiting for cargo check to finish but should have waited 🙈.

@gakonst gakonst merged commit 79f2e1d into gakonst:master Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants