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

Rename node-runtime to node-kitchensink-runtime #11930

Merged

Conversation

wirednkod
Copy link
Contributor

Rename node-runtime to node-kitchensink-runtime in order to emphasize that it is NOTHING production-like, and should not be used, nor inspired from. It is, by all means, a kitchen sink.

Fixes #11814

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jul 28, 2022
@wirednkod wirednkod added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Jul 28, 2022
@wirednkod wirednkod marked this pull request as ready for review July 28, 2022 13:41
@wirednkod wirednkod requested a review from a team as a code owner July 28, 2022 13:41
@github-actions github-actions bot removed the A3-in_progress Pull request is in progress. No review needed at this stage. label Jul 28, 2022
@wirednkod wirednkod force-pushed the nik-rename-node-runtime-to-kitchensink branch from 80d2b7d to 50effb1 Compare July 28, 2022 13:55
@wirednkod
Copy link
Contributor Author

@kianenigma @bkchr @alvicsam I am sorry - but I have to close #11876 in favour of this one. There is something going on wrong with the tests (narrowed it down to merge issue with Cargo.lock) - so I added all my changes to this PR instead.
Again apologies for the mess and please (again) for your review 🙏🏽

@ggwpez
Copy link
Member

ggwpez commented Jul 28, 2022

There is something going on wrong with the tests (narrowed it down to merge issue with Cargo.lock)

You can just ask for help next time. We can easily fix this together 😄
Closing and opening a new MR only gets you rid of the precious approvals.

@wirednkod
Copy link
Contributor Author

You can just ask for help next time. We can easily fix this together 😄 Closing and opening a new MR only gets you rid of the precious approvals.

I know :( I did not want to bother too many people and while testing a new PR I realised that all tests passed, so I figured its maybe better this way. Will keep in mind... Thank you @ggwpez 🙏🏽

docs/Upgrading-2.0-to-3.0.md Outdated Show resolved Hide resolved
docs/Upgrading-2.0-to-3.0.md Outdated Show resolved Hide resolved
bin/node/runtime/Cargo.toml Show resolved Hide resolved
@wirednkod wirednkod requested a review from ggwpez July 28, 2022 19:12
Copy link
Member

@davxy davxy left a comment

Choose a reason for hiding this comment

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

I'm pretty neutral to this renaming, but if the Substrate "example" node runtime is going to be renamed to kitchensink should't all the others related crates (the ones under bin/node) be "codenamed" kithensink?

E.g. node-executor -> kitchensink-executor, node-cli -> kitchensink-cli, etc

@@ -16,7 +16,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

//! Subkey utility, based on node_runtime.
//! Subkey utility, based on kitchensink_runtime.
Copy link
Member

@davxy davxy Jul 29, 2022

Choose a reason for hiding this comment

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

Suggested change
//! Subkey utility, based on kitchensink_runtime.
//! Subkey utility

I don't see any link between this utility and the node (or kitchensink :-D ) runtime

@kianenigma
Copy link
Contributor

I'm pretty neutral to this renaming, but if the Substrate "example" node runtime is going to be renamed to kitchensink should't all the others related crates (the ones under bin/node) be "codenamed" kithensink?

E.g. node-executor -> kitchensink-executor, node-cli -> kitchensink-cli, etc

Good point, but not necessarily. The node part is still a node, but what was previously called the runtime of the node ("node-runtime") should be called kitchensink runtime.

@paritytech-ci paritytech-ci requested a review from a team August 2, 2022 07:35
@kianenigma
Copy link
Contributor

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for d4fdf40

@kianenigma
Copy link
Contributor

@alvicsam do you know what's up with CI?

@alvicsam
Copy link
Contributor

alvicsam commented Aug 2, 2022

yes, one more approve from our team is needed, I'll ping the team

@alvicsam
Copy link
Contributor

alvicsam commented Aug 2, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 43427bd into master Aug 2, 2022
@paritytech-processbot paritytech-processbot bot deleted the nik-rename-node-runtime-to-kitchensink branch August 2, 2022 15:25
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
* Rename node=runtime to kithensink-runtime

* Undo md formatting
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Rename node=runtime to kithensink-runtime

* Undo md formatting
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename node-runtime to something containing kitchen-sink
7 participants