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

Hide remote actor functionality behind "remote" feature #60

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

shusvr
Copy link
Contributor

@shusvr shusvr commented Oct 8, 2024

Closes #58

Not sure if I made all unnecessary dependencies optional, maybe you'll have some suggestions on which other dependencies can be disabled.

@shusvr shusvr changed the title Hide remote functionality before "remote" feature Hide remote actor functionality behind "remote" feature Oct 9, 2024
Copy link
Owner

@tqwewe tqwewe left a comment

Choose a reason for hiding this comment

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

This is great! Thank you for taking the time to do this 😍

I'm not sure that docs.rs will recognise enable a feature = "doc" flag. I belive these annotations should instead be #[cfg_attr(docsrs, ...)] instead based on the docs here:
https://docs.rs/about/builds.

However, it seems like we can just put:

#![cfg_attr(docsrs, feature(doc_auto_cfg))]

And none of the types need to be annotated with:

#[cfg_attr(feature = "doc", doc(cfg(feature = "remote")))]

This cleans things up a lot, making the feature gating in the docs automatically detected!

src/actor/id.rs Outdated Show resolved Hide resolved
src/error.rs Outdated Show resolved Hide resolved
src/request/ask.rs Show resolved Hide resolved
src/request.rs Show resolved Hide resolved
@tqwewe tqwewe added the enhancement New feature or request label Oct 9, 2024
@shusvr
Copy link
Contributor Author

shusvr commented Oct 9, 2024

Actually feature = "doc" would be used by docsrs since I used all-features = true in the manifest, but I do agree doc_auto_cfg is more convenient.

@shusvr shusvr requested a review from tqwewe October 9, 2024 11:30
Copy link
Owner

@tqwewe tqwewe left a comment

Choose a reason for hiding this comment

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

All good, looks great! Thank you again for taking the time to work on this

I also quickly put a couple error types I forgot to mentioned before behind the remote feature flag.

@tqwewe tqwewe merged commit 020dfec into tqwewe:main Oct 9, 2024
1 check passed
tqwewe added a commit that referenced this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add feature flag for distributed actors
2 participants