Allow specifying the subxt crate path for generated code
#664
Conversation
cmichi
force-pushed
the
cmichi-allow-specifying-crate-path
branch
from
11c59c3
to
7e890a3
Compare
| @@ -41,6 +42,7 @@ pub struct CompositeDef { | |||
|
|
|||
| impl CompositeDef { | |||
| /// Construct a definition which will generate code for a standalone `struct`. | |||
| #[allow(clippy::too_many_arguments)] | |||
There was a problem hiding this comment.
Possibly create a follow-up ticket to refactor with a config struct instead of the params.
|
I haven't looked properly at the test you've added yet. One quicker thing you could do is to do all of the codegen with a custom path and check that the string "subxt" doesn't appear at all in it. Otherwise I wonder about tweaking the subxt crate import in the tests to be a custom path; at least if using acustom path is the default thing in the tests it'll get fairly well covered! |
codegen/src/types/derives.rs
Outdated
|
|
||
| pub fn default_with_crate_path(crate_path: &CratePath) -> Self { | ||
| Self { | ||
| default_derives: Derives::default_with_crate_path(crate_path), |
There was a problem hiding this comment.
How about removing the Default impls because we never want to be able to forget to provide a crate path?
This could then just be new or new_with_crate_path
codegen/src/types/derives.rs
Outdated
| impl Default for Derives { | ||
| fn default() -> Self { | ||
| Derives::default_with_crate_path(&CratePath::default()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Should this exist? I think we should make sure crate path is always explicitly provided so we are forced to pass it through always (and thus can't accidentally forget).
There was a problem hiding this comment.
So to be on the same page: would this go through to the user-facing side? I.e. would each user always be required to specify ::subxt as the crate path to use?
|
Looks great overall! I am happy with the tests (defaulting to checking that a custom path works everywhere I think is good enough for now and will almost certainly catch any issues there). Just a couple of wee comments; I think we should remove Default impls for things that rely on a crate path wherever possible just to make it hard for future us to miss threading the crate path through somewhere it's needed. |
Co-authored-by: Andrew Jones <ascjones@gmail.com>
Closes #655.
We need this in ink! for use-ink/ink#1234.
I'm not particularly fond of the testing story for this feature, but I would also not duplicate all the existing tests. Hmm.