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 DidYouMean integration optional #86

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Make DidYouMean integration optional #86

merged 1 commit into from
Jul 29, 2022

Conversation

jbourassa
Copy link
Contributor

The gem won't load when DidYouMean is disabled with
--disable-did_you_mean:

RUBYOPT='--disable-did_you_mean' rake
[...]
NameError:
  uninitialized constant Dry::Container::DidYouMean
# ./lib/dry/container/error.rb:9:in `<class:Container>'

With this commit, the gem loads even though some tests fail.


We run some of our Ruby apps with --disable-did_you_mean for performance reasons, hence the change.

I didn't write any tests for this. The only test I could think of would be something like shelling out to load a new Ruby interpreter without DidYouMean and validating that it doesn't fail:

ruby --disable-did_you_mean -r "dry-container" -e ""

I didn't see a lot of value in such test, but open to feedback or alternative.

The gem won't load when DidYouMean is disabled with
--disable-did_you_mean:

```
RUBYOPT='--disable-did_you_mean' rake
[...]
NameError:
  uninitialized constant Dry::Container::DidYouMean
# ./lib/dry/container/error.rb:9:in `<class:Container>'
```

With this commit, the gem loads even though some tests fail.
@jbourassa jbourassa requested a review from solnic as a code owner July 28, 2022 22:03
@cllns
Copy link
Member

cllns commented Jul 28, 2022

Ah, sorry about that. I wrote the PR that added this feature and I failed to consider DidYouMean not being available for this reason.

@solnic solnic merged commit e1d4df0 into dry-rb:main Jul 29, 2022
@solnic
Copy link
Member

solnic commented Jul 29, 2022

Thanks for the PR. I just released 0.10.1 with your fix.

@jbourassa jbourassa deleted the did-you-mean-optional branch July 29, 2022 12:53
@jbourassa
Copy link
Contributor Author

Thanks a ton @solnic 🙇.

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.

3 participants