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

Feature/hdk prelude #1816

Merged
merged 15 commits into from
Oct 31, 2019
Merged

Feature/hdk prelude #1816

merged 15 commits into from
Oct 31, 2019

Conversation

willemolding
Copy link
Collaborator

@willemolding willemolding commented Oct 30, 2019

PR summary

A nice easy review. Adds a new module called prelude to the HDK. This is a common pattern among Rust crates and it contains the macros and types that are required to effectively use the crate. In many cases this simplifies the use statements in a zome to a single line.

As part of this PR I also migrated the zomes not currently using Rust edition 2018 to use it (only the blog zome). This means that extern crate is no longer required further simplifying the experience of using the HDK.

It also catches a few cases in the HDK macros where it assumed certain types were imported. The imports are now explicit in the macro.

The changes to the HDK are 100% backward compatible!!! So this won't break any existing zome code or examples.

testing/benchmarking notes

( if any manual testing or benchmarking was/should be done, add notes and/or screenshots here )

changelog

Please check one of the following, relating to the CHANGELOG-UNRELEASED.md

  • this is a code change that effects some consumer (e.g. zome developers) of holochain core so it is added to the CHANGELOG-UNRELEASED.md (linked above), with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

documentation

@willemolding willemolding added the WIP work in progress label Oct 30, 2019
@willemolding willemolding removed the WIP work in progress label Oct 30, 2019
/// Types required by all but the most trivial zomes.
/// This can greatly simplify imports for the majority of developers
/// by simply adding use hdk::prelude::*;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is really the only file needed to be reviewed. Please check if you think these types are sufficient or if they are too broad. I made a judgement call to include these ones in particular based on the app spec.

@@ -1,41 +1,10 @@
#![warn(unused_extern_crates)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A great example of why this is awesome

Copy link
Collaborator

@timotree3 timotree3 left a comment

Choose a reason for hiding this comment

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

Overall, I really like the idea of introducing a prelude! I left one nit about a doc comment. I will point out that a prelude with this many things in it (37!) is pretty unusual.

My understanding is that the best practice for preludes is to create one that only re-exports traits, and even then, use a nameless re-export.

For some (admittedly cherrypicked) examples, tokio and std::io.

I'm fine with landing this for now since we are trying to make it easy for rapid prototyping, but in the long game, I'm not so sure that it's the API we want.

crates/hdk/src/prelude.rs Outdated Show resolved Hide resolved
@willemolding willemolding merged commit cac905c into develop Oct 31, 2019
@neonphog neonphog deleted the feature/hdk-prelude branch March 5, 2020 23:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants