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

Upgrade jsonrpsee to v0.4.1 #10022

Merged
merged 17 commits into from
Nov 11, 2021
Merged

Upgrade jsonrpsee to v0.4.1 #10022

merged 17 commits into from
Nov 11, 2021

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Oct 13, 2021 •
edited
Loading

Upgrade remote-externalities to use the latest jsonrpsee version.

q: is there a way to avoid adding the DeserializeOwned bound everywhere? :/ No, unfortunately not, as the Hash is part of the return type.

polkadot companion: paritytech/polkadot#4256

@dvdplm dvdplm requested a review from kianenigma October 13, 2021 18:09
@dvdplm dvdplm self-assigned this Oct 13, 2021
@dvdplm dvdplm added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Oct 13, 2021
niklasad1
niklasad1 approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@niklasad1 niklasad1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

I guess the DeserializeOwned bounds is required because serde couldn't write handwritten one for it

Not sure, if it would possible to restrict B::Hash: DeserializeOwned instead of B: DeserializeOwned.... but most likely a bigger refactor

However, we can't merge this PR until the macos pipeline is updated with a newer rustc version

@dvdplm
Copy link
Contributor Author

dvdplm commented Oct 14, 2021

Not sure, if it would possible to restrict B::Hash: DeserializeOwned instead of B: DeserializeOwned.... but most likely a bigger refactor

I had a stab at replacing the Block: BlockT bound with just Hash. It works, but there doesn't seem to be a HashT equivalent to the BlockT trait so it gets a bit verbose, e.g. Hash: DeserializeOwned + Serialize + Copy + Clone + fmt::Debug + Send + Sync + 'static so I'm not sure it's worth it. (The existing Hash trait could also work, possibly).

@dvdplm dvdplm requested a review from emostov October 19, 2021 08:01
@dvdplm
Copy link
Contributor Author

dvdplm commented Nov 9, 2021

@kianenigma PTAL?

kianenigma
kianenigma approved these changes Nov 9, 2021
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Thanks!

@emostov
Copy link
Contributor

emostov commented Nov 10, 2021

Look like CI is complaining about an unused import https://gitlab.parity.io/parity/substrate/-/jobs/1212818

@dvdplm dvdplm requested a review from andresilva as a code owner November 10, 2021 12:28
@dvdplm dvdplm added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Nov 10, 2021
@dvdplm dvdplm mentioned this pull request Nov 10, 2021
Merged
@dvdplm
Copy link
Contributor Author

dvdplm commented Nov 10, 2021

bot merge

@paritytech-processbot
Copy link

Error:
Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

  • 'Batch: Availability and Validity' does not match any projects in polkadot's Process.json
  • 'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

Merge failed. Check out the criteria for merge. If you're not meeting the approval count, check if the approvers are members of substrateteamleads or core-devs.

@dvdplm
Copy link
Contributor Author

dvdplm commented Nov 11, 2021

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/polkadot#4256 is not mergeable

dvdplm added a commit to paritytech/cumulus that referenced this pull request Nov 11, 2021
@dvdplm
Companion to paritytech/polkadot#4256 and paritytech/substrate#10022
4c96550
@dvdplm dvdplm mentioned this pull request Nov 11, 2021
Closed
@dvdplm
Copy link
Contributor Author

dvdplm commented Nov 11, 2021

bot merge

@paritytech-processbot
Copy link

Error:
Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

  • 'Batch: Availability and Validity' does not match any projects in polkadot's Process.json
  • 'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

Merge failed. Check out the criteria for merge. If you're not meeting the approval count, check if the approvers are members of substrateteamleads or core-devs.

@dvdplm dvdplm merged commit 7b1c81f into master Nov 11, 2021
@dvdplm dvdplm deleted the dp-update-jsonrpsee-client branch November 11, 2021 11:29
ordian added a commit that referenced this pull request Nov 12, 2021
@ordian
Merge branch 'master' into 3x8_update_nightly_fix_fmt
5cf1cc8 
* master: (27 commits)
  Bump rustversion from 1.0.4 to 1.0.5 (#10243)
  Kill the light client, CHTs and change tries. (#10080)
  tuple to struct event variants (#10206)
  Bump thiserror from 1.0.26 to 1.0.30 (#10240)
  Warn about usage of pallet collective set members call. (#10156)
  Bump git2 from 0.13.22 to 0.13.23 (#10238)
  Add group name in task metrics  (#10196)
  Bump proc-macro-crate from 1.0.0 to 1.1.0 (#10237)
  Bump parity-util-mem from 0.10.0 to 0.10.2 (#10236)
  Bump substrate-bip39 from 0.4.2 to 0.4.4 (#10213)
  Upgrade jsonrpsee to v0.4.1 (#10022)
  expose substrate-cli service (#10229)
  Intend to reactivate cargo-unleash check (#10167)
  CI: build docs with deps (#9884)
  use CountedMap in pallet-bags-list (#10179)
  Move all example pallets under `examples` folder. (#10215)
  Upgrade wasm builder (#10226)
  upgrade ss58-registry with additional networks. (#10224)
  move wiki -> docs (#10225)
  new remote-ext mode: (#10192)
  ...
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@dvdplm @niklasad1 @grishasobol
Upgrade jsonrpsee to v0.4.1 (paritytech#10022)
daf6207 
* Upgrade jsonrpsee to v0.4.1

* remove needless BlockT trait bound

* use default wss port in URL

* Fix try_runtime build

* Partially fix for "remote-tests" feature

* Review feedback

* fmt

* Sort out trait bounds for benches

* Fmt

* fmt again?

* fmt with nightly-2021-09-13

* Upgrade try-runtime as well

* fmt

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@dvdplm @niklasad1 @ark0f
Upgrade jsonrpsee to v0.4.1 (paritytech#10022)
8107342 
* Upgrade jsonrpsee to v0.4.1

* remove needless BlockT trait bound

* use default wss port in URL

* Fix try_runtime build

* Partially fix for "remote-tests" feature

* Review feedback

* fmt

* Sort out trait bounds for benches

* Fmt

* fmt again?

* fmt with nightly-2021-09-13

* Upgrade try-runtime as well

* fmt

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@niklasad1 niklasad1 niklasad1 approved these changes

@kianenigma kianenigma kianenigma approved these changes

@emostov emostov emostov approved these changes

@maciejhirsz maciejhirsz Awaiting requested review from maciejhirsz

@andresilva andresilva Awaiting requested review from andresilva

Assignees

@dvdplm dvdplm

Labels
B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dvdplm @emostov @kianenigma @niklasad1