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

Rename some crates for publishing to crates.io #12837

Merged

Conversation

joao-paulo-parity
Copy link
Contributor

@joao-paulo-parity joao-paulo-parity commented Dec 4, 2022

@joao-paulo-parity joao-paulo-parity added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B3-apinoteworthy 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 labels Dec 4, 2022
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Besides the one nitpick, it looks good.

@@ -1,5 +1,5 @@
[package]
name = "remote-externalities"
name = "remote-ext"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name = "remote-ext"
name = "frame-remote-ext"

Or something similar, but it should be more expressive.

CC @kianenigma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other name I've reserved is frame-remote-externalities. I'll change to that, then.

@joao-paulo-parity
Copy link
Contributor Author

joao-paulo-parity commented Dec 5, 2022

Failure of https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2115137#L1544 seems to be related to a bug in check_dependent_project.sh. Let me see if I can fix that. Moving to Draft in the meantime.

@joao-paulo-parity
Copy link
Contributor Author

I think the fix was effective as per https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2116743#L1811 which didn't fail to compile polkadot-node-subsystem-test-helpers like https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2115137#L1544. Now it's failing with a different error which appears to be unrelated to this PR:

Checking cumulus-relay-chain-inprocess-interface v0.1.0 (/builds/parity/mirrors/substrate/companions/cumulus/client/relay-chain-inprocess-interface)
error[E0308]: mismatched types
  --> client/relay-chain-minimal-node/src/network.rs:75:4
   |
75 | /             Some(Box::new(move |fut| {
76 | |                 spawn_handle.spawn("libp2p-node", Some("networking"), fut);
77 | |             }))
   | |_______________^ expected struct `Box`, found enum `std::option::Option`
   |
   = note: expected struct `Box<(dyn std::ops::Fn(Pin<Box<(dyn futures::Future<Output = ()> + std::marker::Send + 'static)>>) + std::marker::Send + 'static)>`
                found enum `std::option::Option<Box<[closure@client/relay-chain-minimal-node/src/network.rs:75:18: 75:28]>>`

I'll try to fix that.

@joao-paulo-parity
Copy link
Contributor Author

joao-paulo-parity commented Dec 5, 2022

The "different error" I mentioned in #12837 (comment) is very likely unrelated to this PR. I found a fix for it in paritytech/cumulus@747400a, i.e. it's related to #12646.

Fixed as per https://gitlab.parity.io/parity/mirrors/substrate/builds/2117207

@@ -19,7 +19,7 @@ log = "0.4"
parking_lot = "0.12.1"
thiserror = "1.0"
wasm-timer = "0.2.5"
beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy" }
beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy", package = "sp-beefy" }
Copy link
Contributor

@acatangiu acatangiu Dec 5, 2022

Choose a reason for hiding this comment

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

nit:

Why not go full rename from the get-go?

Suggested change
beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy", package = "sp-beefy" }
sp-beefy = { version = "4.0.0-dev", path = "../../primitives/beefy" }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would incur touching the .rs files as well, right? I followed @athei 's suggestion from https://forum.parity.io/t/new-names-suggestions-for-squatted-crates/1513/2 of only changing the Cargo.tomls.

I went with this approach because it minimizes the amount of changes I have to do in this PR and the companions. However, I'm open to whatever approach is the most agreed upon. Changing the .rs files wouldn't be difficult, it'd just be more work. Also it might require more reviews from other teams.

Do you strongly prefer the full rename or is it acceptable to leave it like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there's some hurry with this PR, I would prefer the full rename, otherwise you can kick the can down the road and someone will do it later; not blocking the PR on this :)

Copy link
Member

Choose a reason for hiding this comment

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

I think we are a bit in a hurry with it; since we want to release the crates ASAP.

@joao-paulo-parity
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 416a331 into paritytech:master Dec 6, 2022
@joao-paulo-parity joao-paulo-parity deleted the rename-crates branch December 6, 2022 11:42
coderobe pushed a commit that referenced this pull request Dec 6, 2022
* rename some crates for publishing to crates.io

* s/remote-ext/frame-remote-externalities
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* rename some crates for publishing to crates.io

* s/remote-ext/frame-remote-externalities
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* rename some crates for publishing to crates.io

* s/remote-ext/frame-remote-externalities
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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
Development

Successfully merging this pull request may close these issues.

6 participants