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

Update for latest dry-configurable setting API #686

Merged
merged 6 commits into from
Sep 12, 2021

Conversation

timriley
Copy link
Member

No description provided.

@timriley timriley requested a review from solnic as a code owner May 24, 2021 12:47
@timriley timriley force-pushed the update-for-latest-dry-configurable branch from f332067 to 58ea332 Compare May 24, 2021 12:56
@timriley timriley changed the title Update for latest dry-configurable setting API, use dry/core/equalizer Update for latest dry-configurable setting API May 24, 2021
Copy link
Member

@solnic solnic left a comment

Choose a reason for hiding this comment

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

I think I want to make it work with both versions until 2.0.0. Thoughts?

@timriley
Copy link
Member Author

@solnic good thought, and I think you're right, particularly for the post-1.0 gems of ours. Do you have any thoughts about how to go about it?

@solnic
Copy link
Member

solnic commented May 26, 2021

@timriley I did this here 5ff1883 but also did this there 0471911aa95a184f594cfded07f762becad09489 so if we require latest dry-schema here then what I did here won't be needed

EDIT 3: fixing link to dry-schema commit 🙄

@timriley
Copy link
Member Author

@solnic I like what you did over in 0471911 (dry-schema #356) — would you be happy with that approach here too?

@solnic
Copy link
Member

solnic commented May 26, 2021

@timriley yes sorry that's what I meant 🙂 the only "issue" is that we would have to ensure the new dry-validation depends on the new dry-schema too...unless you want to override setting here as well?

@timriley
Copy link
Member Author

@solnic Yeah, I think we'd want to put this shim directly in every gem that uses such an approach

@solnic
Copy link
Member

solnic commented May 27, 2021

@timriley done here c8ad125

@timriley timriley force-pushed the update-for-latest-dry-configurable branch from 4c11707 to 8608f48 Compare August 5, 2021 02:05
This shouldn't be needed with the improved backwards compatibility expected in dry-configurable 0.13.0
@timriley
Copy link
Member Author

timriley commented Sep 12, 2021

Thanks to the improved backwards compatibility in the upcoming dry-configurable deprecations (tested in dry-rb/dry-configurable#120), this one no longer needed any kind of additional shim.

@timriley timriley merged commit b6ee512 into master Sep 12, 2021
@timriley timriley deleted the update-for-latest-dry-configurable branch September 12, 2021 10:42
@timriley timriley mentioned this pull request Sep 12, 2021
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