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

Remove dry-validation from dependencies #317

Closed
wants to merge 3 commits into from

Conversation

BuonOmo
Copy link
Contributor

@BuonOmo BuonOmo commented Apr 26, 2022

Fixes #276

Problem

Having the dependency on a fix dry-validation version generates multiple issues:

  • users who want another version of the dry-validation gem are blocked
  • the dry-validation gem itself has dependencies, making the package size bigger, even if not needed.

Solution

We remove the dependency, but still check at runtime that the dependency is met if and only if the schema validation feature is used.

Other aspects

All of the changes described below are in a separate commit, I can revert it if it helps merging!

Ruby 2.1 compatibility

During the implementation, I had a bug with Config reloading. The lib/config/compatibility.rb introduces a bug that is really hard to debug (for me at least):

# in config.gemspec
require_relative "config/version" # => true

# somewhere after
p Config.const_defined?(:CONFIG) # => true
require "config" # => true
# will remove Config and then require everything... Except what was 
# already required, e.g. "config/version"
p Config.const_defined?(:CONFIG) # => false

Since this file was added for an EOL ruby version (2.1, per commit c4119fb), I removed it.

Usage of require_relative

Helps be sure that we talk about the same file! Otherwise, it depends on $LOAD_PATH (or $:)

Dir[..].sort.each { require }

Make sure that the require order is not OS, or filesystem dependent

.ruby-version

I've removed the file, as for Gemfile.lock, the ruby version will in the end depend on the person using the gem, so developers of the gem should also be able to run it with their local ruby versions!

Config::Error

Since there are now two possible errors, lets regroup those under the same banner! (not that it is not a breaking change since .is_a? will still behave the same)

@teohm
Copy link

teohm commented Aug 24, 2022

What's the next step for this PR? We are waiting for the release of this PR, so that we can use RubyConfig in our production apps. Anything we can help to move the PR to the next stage?

Some context: Our production apps are using an older version of dry-validation gem.

@BuonOmo
Copy link
Contributor Author

BuonOmo commented Aug 24, 2022

@teohm same here, I think the most important thing needed is traction and a maintainer discussing it. If you think it needs something from my end as well, please ping me.

I guess you could also use this branch, which would prove that it works fine :)
FYI, that is what we do at Stuart

@BuonOmo
Copy link
Contributor Author

BuonOmo commented Apr 2, 2023

@pkuczynski @cjlarose sorry for the noise, but should I update the PR, do you think you'd be able to review it any time?

@cjlarose
Copy link
Member

cjlarose commented Apr 2, 2023

@BuonOmo Sorry I dropped the ball on this. This looks great!

What is the experience if someone specifies a validation_contract instead of using config.schema as described in the README? Is a runtime check still performed to ensure we're using a compatible version?

@BuonOmo
Copy link
Contributor Author

BuonOmo commented Apr 3, 2023

Closed in favor of #333

@BuonOmo BuonOmo closed this Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Split validation via dry-rb into seperate gem
3 participants