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

update most of the dependencies #1946

Merged
merged 12 commits into from
Nov 17, 2020
Merged

update most of the dependencies #1946

merged 12 commits into from
Nov 17, 2020

Conversation

ordian
Copy link
Member

@ordian ordian commented Nov 11, 2020

The update was done via cargo upgrade --workspace and then a couple of updates were reverted:

  • tiny-keccak was updated manually

as mentioned in paritytech/substrate#7509 (comment), it would be nice to do a burnin.

@ordian ordian added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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. A1-needsburnin labels Nov 11, 2020
@ordian
Copy link
Member Author

ordian commented Nov 13, 2020

Haven't noticed any anomalies in the metrics.

mxinden
mxinden reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @ordian!

For what my review is worth, the changes look good to me, apart from the question below. Given my low involvement in the Polkadot code base and the size of this change, I would very much appreciate more eyes on this pull request.

Haven't noticed any anomalies in the metrics.

Same here. This has been running on a sentry node (kusama-sentry-ew4-0) for more than 5 days. Before merging, I would recommend running it on a validator as well. What do you think @ordian?

node/core/candidate-validation/Cargo.toml Outdated
log = "0.4.11"

sp-core = { package = "sp-core", git = "https://github.com/paritytech/substrate", branch = "master" }
parity-scale-codec = { version = "1.3.0", default-features = false, features = ["bit-vec", "derive"] }
parity-scale-codec = { version = "1.3.5", default-features = false, features = ["bit-vec", "derive"], package = "parity-scale-codec" }
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand package correctly, one can use it to rename dependencies. Why is it needed here, given that it does not rename the import variable, but instead just repeats the name?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was a bug in cargo-edit tool that I used to automatically bump all the dependencies. Reverted this change manually for futures in 3d16db4, thanks for catching this for parity-scale-codec, will fix now.

tomaka
tomaka reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@tomaka tomaka 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.

There's indeed a lot of these package = "...", notably for futures and parity-scale-codec.

LGTM once these are removed.
I'm neutral about the question of whether to test it on a validator as well.

@ordian
remove all renames on parity-scale-codec
65a5f59
@ordian
Merge branch 'master' into ao-update-dependencies
bb1c419 
* master:
  Make sure adder collator test is executed in CI (#1949)
  Restrict collating_using_adder_collator test on feature real-overseer (#1947)
@ordian
remove the leftovers
4a36454
@ordian
Copy link
Member Author

ordian commented Nov 16, 2020

Addressed the renaming for parity-scale-codec and the remaining for futures.

Note that this PR doesn't upgrade dependencies constrained by sub-dependencies, like libp2p (constrained by substrate) or bitvec (by parity-scale-codec). Once we upgrade dependencies in substrate, there might be a new bunch to upgrade here.

@ordian
Copy link
Member Author

ordian commented Nov 16, 2020

Before merging, I would recommend running it on a validator as well. What do you think @ordian?

I don't think a validator burn-in is necessary here, but don't have a strong opinion either.

mxinden
mxinden approved these changes Nov 16, 2020
View reviewed changes
Copy link
Contributor

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

For what my review is worth, this looks good to me.

@ordian
remove unused dep
d44045b
@ordian
Merge branch 'master' into ao-update-dependencies
247613e 
* master:
  Breakdown the Router module on Dmp, Ump, Hrmp modules (#1939)
  Add CI job to verify extrinsic ordering (#1950)
@ordian ordian merged commit 31fb8fe into master Nov 17, 2020
@ordian ordian deleted the ao-update-dependencies branch November 17, 2020 10:16
This was referenced Nov 17, 2020
Merged
Merged
@ordian ordian mentioned this pull request Nov 18, 2020
Merged
16 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomaka tomaka tomaka approved these changes

@mxinden mxinden mxinden approved these changes

Assignees
No one assigned
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ordian @tomaka @mxinden