Skip to content
This repository was archived by the owner on Oct 13, 2025. It is now read-only.

Use dry-inflector#482

Merged
jodosha merged 3 commits intohanami:unstablefrom
artofhuman:use-dry-inflector
Jun 4, 2018
Merged

Use dry-inflector#482
jodosha merged 3 commits intohanami:unstablefrom
artofhuman:use-dry-inflector

Conversation

@artofhuman
Copy link
Contributor

Closes #481

@codecov
Copy link

codecov bot commented Apr 19, 2018

Codecov Report

Merging #482 into unstable will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           unstable     #482      +/-   ##
============================================
+ Coverage     98.73%   98.73%   +<.01%     
============================================
  Files           108      108              
  Lines          5513     5527      +14     
============================================
+ Hits           5443     5457      +14     
  Misses           70       70
Impacted Files Coverage Δ
spec/unit/hanami/model/configuration_spec.rb 98.18% <100%> (+0.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18bfa10...b36fcf3. Read the comment docs.

Copy link
Member

@jodosha jodosha left a comment

Choose a reason for hiding this comment

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

@artofhuman Thanks for this PR, but there is a change to be done before to merge. My apologize for the wrong description in #481


Given Hanami::Model::Configuration isn't directly instantiated, but it's done indirectly via:

Hanami::Model.configure do
  # ...
end

We need to find a way for .configure to accept a custom inflector and/or custom inflections.

@jodosha jodosha self-assigned this Apr 24, 2018
@jodosha jodosha added this to the v2.0.0 milestone Apr 24, 2018
@artofhuman
Copy link
Contributor Author

artofhuman commented Apr 25, 2018

@jodosha thanks for review, i`ll try to implement

@jodosha
Copy link
Member

jodosha commented Apr 26, 2018

@artofhuman Please let me know in case you'll need help. Thanks. 😃

@artofhuman artofhuman changed the title Use dry-inflector Use dry-inflector [wip] Apr 27, 2018
@artofhuman artofhuman changed the title Use dry-inflector [wip] Use dry-inflector May 3, 2018
@artofhuman
Copy link
Contributor Author

@jodosha could you check?

Copy link
Member

@jodosha jodosha left a comment

Choose a reason for hiding this comment

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

@artofhuman Thanks it looks better now. I'd call for a second review, if you don't mind. 😃

@jodosha jodosha requested a review from mereghost May 7, 2018 16:45
@artofhuman
Copy link
Contributor Author

@jodosha ok ;)

@jodosha jodosha merged commit 7ed11de into hanami:unstable Jun 4, 2018
@jodosha jodosha mentioned this pull request Jun 4, 2018
2 tasks
@cllns
Copy link
Member

cllns commented Jun 4, 2018

Already merged but looks fine to me too ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants