-
Notifications
You must be signed in to change notification settings - Fork 2
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
Use dry-inflector, because Inflecto is no longer maintained #36
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 36 36
Lines 740 747 +7
=====================================
+ Hits 740 747 +7
Continue to review full report at Codecov.
|
…except #inspect
Trying to remove using Inflector class methods…
Not really sure where to put this
module Lurch | ||
class Inflector | ||
def initialize(inflection_mode, types_mode) | ||
@inflector = Dry::Inflector.new do |inflections| | ||
inflections.plural "people", "people" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this configuration required here? Would we need a way to expose custom pluralization rules to the end-user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was basically added only to make the tests run. Apparently dry-inflector does pluralize a little different than Inflecto.
Yes, customizing the Inflector seems to be necessary.
However, I think we should also reduce the number of calls to a minimum, because this seems to be a big performance hit (#37) or remove the inflector completely. Both seems to be out of scope for this PR.
This was done in two steps:
1 is good, because that way we can have different inflector settings per Store. 🤷♂️