refactor!: drop six runtime dependencies - #5
Merged
Merged
Conversation
The gemspec declared eight runtime dependencies. Six of them were never
loaded by this gem:
* dry-initializer, used for five initializers that plain Ruby writes
just as short. Its readers were private, so replacing them with
private attr_readers leaves the public API untouched.
* connection_pool and redis, whose constants appear nowhere in lib/.
The pool is duck typed on #with, and Redis::Namespace comes from
redis-namespace.
* deepl-rb, required at load time but never referenced. The API object
is supplied by the application through DeepLDiff.api.
* redis-namespace and ratelimit, named only inside method bodies of
the two optional Redis wrappers, so they already resolved lazily.
Only ox and punkt-segmenter remain, which is what the README promised
all along: "This dependencies are not included, as you might need to
roll your own cache based on different store."
Fix RedisRateLimiter while here. threshold and interval were positional
params, so the keyword call the README documents silently fell back to
8000 and 60. They are keywords now, matching RedisCacheStore.
Cover both Redis wrappers, which had no tests at all. The stand-ins they
use for Redis::Namespace, Ratelimit and the connection pool double as an
executable statement of the duck types this gem expects.
BREAKING CHANGE: applications that relied on this gem to install the
DeepL client, redis, redis-namespace, connection_pool or ratelimit must
now depend on them directly. Callers passing threshold and interval to
RedisRateLimiter positionally must switch to keywords.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The gemspec declared eight runtime dependencies. Six of them are never loaded by this gem. After this change it loads two:
oxandpunkt-segmenter.That is what the README promised all along — "This dependencies are not included, as you might need to roll your own cache based on different store" — the gemspec just never agreed with it.
What was actually used
lib/oxOx::Sax,Ox.sax_parsein the tokenizerpunkt-segmenterPunkt::SentenceTokenizerin the tokenizerdry-initializerextendlinesconnection_pool#withredisRedisis never nameddeepl-rbrequire "deepl"at load, constant unusedredis-namespaceRedisCacheStoreratelimitRedisRateLimiterThe last two already resolved lazily: they are named inside method bodies, so the gem loaded fine without them and only broke if you actually used those classes. Declaring them as hard runtime dependencies forced them on everyone who used their own cache store.
dry-initializer → plain Ruby
Dry::Initializergenerated private readers, so replacing them with a privateattr_readerkeeps the public API byte-for-byte identical:Bug fixed on the way
RedisRateLimiterdeclaredthresholdandintervalas positionalparams, so the keyword call the README documents silently discarded them:They are keywords now, matching
RedisCacheStore. There is a regression test.Tests
Both Redis wrappers had no tests at all. They have them now — 6 new cases. The stand-ins they use for
Redis::Namespace,Ratelimitand the connection pool double as an executable statement of the duck types this gem expects.Coverage: 94.68% → 97.57% (282/289).
Breaking changes → 2.0.0
redis,redis-namespace,connection_poolorratelimitmust depend on them directly. The README now says so explicitly and its example requiresdeepl.threshold/intervaltoRedisRateLimiterpositionally must switch to keywords.Test plan
bundle exec rake test— 34 runs, 44 assertions, 0 failures, on Ruby 4.0.5 and 3.2.4.bundle exec rubocop— 0 offenses.Gemfile.lockre-resolved: zero matches fordry-initializer,deepl-rb,redis,connection_pool,ratelimit. With none of them installed,require "deepl_diff"loads andDeepLDiff::Tokenizer.tokenize("<b>hi</b>")returns the expected tokens.https://claude.ai/code/session_01Pda49PcgziFVnibkKKjXRE