Skip to content

Move model crate stubs into defaults #2235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

fhill2
Copy link
Contributor

@fhill2 fhill2 commented Jan 23, 2025

Pull Request

If a rust project depends on nautilus_model crate, the project only compiles by including the "stubs" feature.

This is due to the Default trait impls being included in the production execution path, while also being gated behind the "stubs" feature.

The minimal repro for this is building a rust project with the nautilus-model crate as a single dependency.

I've provided a fix that moves the trait impls into either of these locations:

  • <module_name>/default.rs -> any code used in production for derive(Default) unwrap_or_default()
  • <module_name>/stubs.rs -> anything used in the test execution path

Type of change

Delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How has this change been tested?

Test 1:

cargo build in a rust Project that depends on crate nautilus_model compiles successfully with no additional features.

nautilus-model = { path = "./nautilus_trader/nautilus_core/model"}

Test 2:

make install-debug builds successfully.

@CLAassistant
Copy link

CLAassistant commented Jan 23, 2025

CLA assistant check
All committers have signed the CLA.

@cjdsellers
Copy link
Member

Hi @fhill2

Good to hear for you. Thanks for the contribution, I think this partitioning is much cleaner 👍
I'll do another pass of the stubs feature as well.

@cjdsellers cjdsellers merged commit bce92d8 into nautechsystems:develop Jan 23, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants