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

Make gethostname an optional feature #25

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

daniel-levin
Copy link
Contributor

@daniel-levin daniel-levin commented Jul 10, 2024

Hi there @mdecimus!

I am a happy user of several of your email crates, including mail-builder. I want to be able to use mail-builder without depending on gethostname. I propose making the dependency on gethostname optional, but enabled by default, so that existing users of this crate are not disrupted in any way.

This change has three benefits:

  1. It reduces the size of the dependency graph.
  2. MessageBuilder never blocks, so it's usable inside an async context.
  3. My outputs become deterministic and aren't tied to the machine they're generated on. This is important for my tests in my network-jailed CI system.

While the usage of #[cfg(...)] inline is a bit awkward, I found it was the simplest way to achieve my goal. The alternatives were:

  1. Using a macro, which means you have to navigate elsewhere to see what the code is really doing.
  2. Needlessly copying the &str returned by gethostname.

@mdecimus mdecimus merged commit ae97759 into stalwartlabs:main Jul 10, 2024
1 check passed
@mdecimus
Copy link
Member

Thanks!

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.

2 participants