This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Attempting to fix publishing #9140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
A3-in_progress
gnunicorn
added
A0-please_review
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`
TriplEight
reviewed
Jun 18, 2021
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`
gnunicorn
requested review from
andresilva,
athei,
kianenigma,
NikVolf,
sorpaas and
tomusdrw
as code owners
Xanewok
force-pushed
the
ben-fixing-for-unleashing
branch
from
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 😃 |
ordian
reviewed
Jul 7, 2021
| - 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.
why not put in the variables: section above?
There was a problem hiding this comment.
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.
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.
ordian
reviewed
Jul 7, 2021
| 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.
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.
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.
ordian
approved these changes
Jul 7, 2021
|
Merged master and the checks are green again; it'd be great to get one more approval and let's try merging it then |
sorpaas
approved these changes
Jul 11, 2021
|
bot merge |
|
Trying merge. |
ghost
merged commit 
|
Cheers! |
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This sort of works locally, let's see what the CI thinks about that.
Fixes #9129
//cc @Xanewok