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

Validations Autoload #2207

Merged
merged 4 commits into from
Dec 27, 2021
Merged

Conversation

ericproulx
Copy link
Contributor

Hello,

I saw that almost all classes/modules were autoloaded but the Validations tree wasn't. Anyway, here's a PR with the autoloading. Merry Xmas! 🎄

@grape-bot
Copy link

grape-bot commented Dec 25, 2021

1 Warning
⚠️ There're library changes, but not tests. That's OK as long as you're refactoring existing code.

Generated by 🚫 Danger

@dblock
Copy link
Member

dblock commented Dec 27, 2021

Thank you! Add to CHANGELOG please?

There's a couple of bugs that mention auto-load, does any of this fix that? https://github.com/ruby-grape/grape/issues?q=is%3Aissue+validations+auto-load+is%3Aopen

@ericproulx
Copy link
Contributor Author

Thank you! Add to CHANGELOG please?

There's a couple of bugs that mention auto-load, does any of this fix that? https://github.com/ruby-grape/grape/issues?q=is%3Aissue+validations+auto-load+is%3Aopen

I'm not sure it will fix the issues. I think the best way to load any custom validators would be through the Grape::Validations.register_validator function which is used within specs. I've discovered that function through this PR and it's not mention in the documentation.

Maybe having a lazy load hook would help to clarify when to load custom validators. Something like:

# /initializers/grape.rb
ActiveSupport.on_load(:grape). do # would need a 
  require 'my_custom_validator'
  Grape::Validations.register_validator('my_custom_validator', MyCustomValidator)
end

@dblock dblock merged commit 6d417fa into ruby-grape:master Dec 27, 2021
@ericproulx ericproulx mentioned this pull request Dec 27, 2021
@dblock
Copy link
Member

dblock commented Dec 27, 2021

@ericproulx I think the JRuby failure is related to this:

Run bundle exec rake spec
/home/runner/.rubies/jruby-head/bin/jruby -I/home/runner/work/grape/grape/vendor/bundle/jruby/3.0.0/gems/rspec-core-3.10.1/lib:/home/runner/work/grape/grape/vendor/bundle/jruby/3.0.0/gems/rspec-support-3.10.3/lib /home/runner/work/grape/grape/vendor/bundle/jruby/3.0.0/gems/rspec-core-3.10.1/exe/rspec --pattern spec/\*\*/\*_spec.rb  --exclude-pattern spec/integration/\*\*/\*_spec.rb
/home/runner/.rubies/jruby-head/lib/ruby/stdlib/time.rb:3: warning: parentheses after method name is interpreted as an argument list, not a decomposed argument

An error occurred while loading ./spec/grape/api/custom_validations_spec.rb. - Did you mean?
                    rspec ./spec/grape/exceptions/validation_spec.rb
                    rspec ./spec/grape/dsl/validations_spec.rb
                    rspec ./spec/grape/validations_spec.rb***

Check out the truffleruby and irb head failures too, maybe easy to fix.

@dm1try
Copy link
Member

dm1try commented Dec 27, 2021

Something like:
Grape::Validations.register_validator('my_custom_validator', MyCustomValidator)

I don't recommend doing this while we have the same call in inherited callback in Base class

dm1try added a commit to dm1try/grape that referenced this pull request Dec 30, 2021
ericproulx pushed a commit to ericproulx/grape that referenced this pull request Jan 3, 2022
@ericproulx ericproulx mentioned this pull request Nov 13, 2023
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.

4 participants