-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conversation
97e0a53
to
a360fca
Compare
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.
Just one nit, otherwise LGTM!
Is this one flaky test or something really broke ?
thread 'tests::import_approval_bad' panicked at 'msg send timeout', node/network/approval-distribution/src/tests.rs:86:10
Co-authored-by: Andrei Sandu <[email protected]>
Flaky test timeouts I assume, since no functionality was changed |
Why should they be published? What is the reasoning behind this? |
It's been a long sought milestone, to give the "pattern" a clear interface boundary and not interleave polkadot specifics with the guts of the generated We don't want to repeat past mistakes, publishing is stage one, there should be a discussion to separate it into it's own repo eventually - but that's out of scope for this PR. |
I'm not sure how publishing will help with this. How often will this be updated on crates.io? How will take care to update it? What is the benefit of having it on crates.io? I mean if you want to separate it, fine, but crates.io doesn't solve this magically. Or is the expected working style then to first publish and integrate the changes in a new pr? |
Publishing was a long standing nice-to-have goal ( CC @rphmeier ) since the it's actually completely independent of any polkadot specifics. @vstakhov will be taking care of the maintenance, and I'll be available for code review on for the foreseeable future.
This has yet to be discussed, but in my books, that would be desirable mid-term. |
Orchestra is a pretty stable component at this point, and as Bernhard says it deliberately avoids Polkadot specifics. This is code which doesn't need to change very often, although there will be a few changes over the next months as we work on optimizations in subsystem communication. IMO we will really want this to be a library for the community to build parachains whose nodes also do custom DB and networking logic. I don't think publishing it on crates.io will cause any major issues, so we might as well. |
Prepare it to be published as an independent crate (all crates are reserved to avoid the squatting pitfal).
metered-channel
->prioritized-metered-channel
in prep for Avoid using unbounded channels for prioritization polkadot-sdk#8240.0.1
forprioritized-metered-channel
,orchestra
andorchestra-proc-macro
metered = { package = "prioritized-metered-channel", .. }
consistentlyNote publishing with unleash is currently borked due to paritytech/cargo-unleash#77 , so the following should work but currently does not. One has to publish them one by one