-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
error[E0369]: binary operation `==` cannot be applied to type `syn::Path` --> src/lib.rs:88:29 | 88 | .filter(|attr| attr.path == parse_quote!(max_encoded_len_crate)) | --------- ^^ ----------------------------------- _ | | | syn::Path error: aborting due to previous error For more information about this error, try `rustc --explain E0369`. Error: could not compile `max-encoded-len-derive`
cargo unleash version bump-breaking --changed-since v3.0.0 cargo unleash version set-pre dev --changed-since v3.0.0 FIXME: Don't modify crates that are not yet released, e.g. `max-encoded-len-derive`
d5e44ca
to
583547c
Compare
Otherwise we run into `Cycle detected` error.
The This takes a crude approach of detecting which packages have changed since It's worth noting that this does not bump every crate to a unified |
Needed for Polkadot
Needed for Polkadot
Right now @s3krit is there a way to test everything in tandem and/or ignore the polkadot companion run if everything is checked manually beforehand? Should we wait for #9010 or is it more about making processbot smarter somehow? |
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
This is now ready for review. #9140 (comment) is addressed by explicitly running
Polkadot companion check job: https://gitlab.parity.io/parity/substrate/-/jobs/1003905
It'd be good to land that sooner rather than later. Firstly, it's prone to bitrotting as any change to a Cargo.toml in the meantime probably requires this PR to be updated. Secondly, it'd be good to finally get the CI green for master and to mark the Took this over from @gnunicorn so I can approve it on his behalf if it's well-reviewed 😃 |
- cargo unleash de-dev-deps | ||
# Reuse build artifacts when running checks (cuts down check time by 3x) | ||
- mkdir -p target/unleash | ||
- export CARGO_TARGET_DIR=target/unleash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not put in the variables:
section above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a makeshift optimization that needs addressing in the tool itself and I didn't want to "properly" encode it here. Should I move it above for consistency for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it's fine to leave it as is then with the comment it's going to removed once cargo unleash addresses the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note
sp-core = { default-features = false, version = "3.0.0", path = "../../../../primitives/core" } | ||
sp-io = { default-features = false, version = "3.0.0", path = "../../../../primitives/io" } | ||
sp-runtime = { default-features = false, version = "3.0.0", path = "../../../../primitives/runtime" } | ||
sp-core = { default-features = false, version = "4.0.0-dev", path = "../../../../primitives/core" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make sense to omit versions for dev-deps only leaving path as you did in paritytech/parity-common#552 (comment)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's something that I have discovered recently and will probably impact the unleash tool as well. As of now, cargo unleash
automatically removes dev-dependencies to break these cycles but if there is 1st party support for that in Cargo than we should rethink what cargo unleash
does and probably migrate to path
-only dev-dependencies in the Substrate repository in the near future.
We talked with @gnunicorn already about that and we will address that somehow but for now I would prefer not to touch that in this PR.
Merged master and the checks are green again; it'd be great to get one more approval and let's try merging it then |
bot merge |
Trying merge. |
Cheers! |
This sort of works locally, let's see what the CI thinks about that.
Fixes #9129
//cc @Xanewok