Skip to content
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

Fix the build problem of wash-lib with --no-default-features flag. #1525

Merged

Conversation

rimbi
Copy link
Contributor

@rimbi rimbi commented Feb 15, 2024

Feature or Problem

Fixes the build problem of wash-lib with --no-default-features flag.

Related Issues

Fixes issue #862

Release Information

N/A

Consumer Impact

N/A

Testing

build succeeds in crates/wash-lib: cargo build --no-default-features

Unit Test(s)

N/A

Acceptance or Integration

N/A

Manual Verification

N/A

Signed-off-by: Cem Eliguzel <cemeliguzel@gmail.com>
@rimbi rimbi requested a review from a team as a code owner February 15, 2024 08:14
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

Thanks for filing this @rimbi ! I just had a few requests to see if we needed to gate as many modules behind the NATS feature flag. Let me know if my assumption is incorrect.

Otherwise, my only other request is to update the module documentation at the top of lib.rs to detail the new modules that are behind the nats feature flag. Looking forward to it!

crates/wash-lib/src/lib.rs Show resolved Hide resolved
crates/wash-lib/src/lib.rs Show resolved Hide resolved
crates/wash-lib/src/lib.rs Show resolved Hide resolved
@rimbi
Copy link
Contributor Author

rimbi commented Feb 16, 2024

Thanks for filing this @rimbi ! I just had a few requests to see if we needed to gate as many modules behind the NATS feature flag. Let me know if my assumption is incorrect.

Otherwise, my only other request is to update the module documentation at the top of lib.rs to detail the new modules that are behind the nats feature flag. Looking forward to it!

Hi @brooksmtownsend . What do you mean with to detail exactly? Listing them in the doc? It may cause a duplication.

@brooksmtownsend
Copy link
Member

Hey @rimbi all I mean is at the top of wash-lib's src/lib.rs there's this comment:

//! | Feature Name | Default Enabled | Description |
//! | --- | --- | --- |
//! | start | true | Contains the [start](start) module, with utilities to start wasmCloud runtimes, NATS, and wadm |
//! | parser | true | Contains the [parser](parser) module, with utilities to parse `wasmcloud.toml` files |
//! | cli | false | Contains the build, cli, and generate modules with additional trait derives for usage in building CLI applications |
//! | nats| true| Contains the [app](app) module with a dependency on `async_nats` |

I just wanted to add the other modules to the list by the nats flag along with app to match the list of modules there. Does that make sense

Signed-off-by: Cem Eliguzel <cemeliguzel@gmail.com>
@rimbi rimbi force-pushed the rimbi-wash-lib-build-no-default-features branch from 035a51e to 8fbd806 Compare February 19, 2024 10:37
@rimbi
Copy link
Contributor Author

rimbi commented Feb 19, 2024

Hey @rimbi all I mean is at the top of wash-lib's src/lib.rs there's this comment:

//! | Feature Name | Default Enabled | Description |
//! | --- | --- | --- |
//! | start | true | Contains the [start](start) module, with utilities to start wasmCloud runtimes, NATS, and wadm |
//! | parser | true | Contains the [parser](parser) module, with utilities to parse `wasmcloud.toml` files |
//! | cli | false | Contains the build, cli, and generate modules with additional trait derives for usage in building CLI applications |
//! | nats| true| Contains the [app](app) module with a dependency on `async_nats` |

I just wanted to add the other modules to the list by the nats flag along with app to match the list of modules there. Does that make sense

done! 👍

@brooksmtownsend brooksmtownsend enabled auto-merge (rebase) February 21, 2024 11:24
@rimbi
Copy link
Contributor Author

rimbi commented Feb 21, 2024

@brooksmtownsend is the failing test a glitch, or?

@brooksmtownsend brooksmtownsend merged commit 1d53c2e into wasmCloud:main Feb 21, 2024
35 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.

None yet

2 participants