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

Don't suppress regular ArgumentError exceptions #366

Merged
merged 4 commits into from
Jul 27, 2022

Conversation

splattael
Copy link
Contributor

Account specifically for the following scenario:

  expose :foo, &:bar

Note that :bar.to_proc.parameters always returns [[:req], [:rest]].

We should not swallow ArgumentError exceptions in any other instance.

See spec for an example.

Account specifically for the following scenario:

  expose :foo, &:bar

Note that `:bar.to_proc.parameters` always returns `[[:req], [:rest]]`.

We should not swallow ArgumentError exceptions in any other instance.
@splattael
Copy link
Contributor Author

👋 @LeFnord Do you mind reviewing this PR? 🙏

Copy link
Member

@LeFnord LeFnord left a comment

Choose a reason for hiding this comment

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

👍 … that makes sense … good improvement

@LeFnord
Copy link
Member

LeFnord commented Jul 26, 2022

@splattael … please, can you run bundle exec rubocop --auto-gen-config

@splattael
Copy link
Contributor Author

@LeFnord Whoops, done ✔️ Thanks for the heads-up 🙇

@LeFnord
Copy link
Member

LeFnord commented Jul 26, 2022

avoid calling of SimpleCov stuff

and you can also change ruby-version: ['2.6', '2.7', '3.0', head, jruby, truffleruby] into ruby-version: ['2.6', '2.7', '3.1', head, jruby, truffleruby]

@splattael
Copy link
Contributor Author

@LeFnord

avoid calling of SimpleCov stuff

What do you mean by this? 🤔

and you can also change ruby-version: ['2.6', '2.7', '3.0', head, jruby, truffleruby] into ruby-version: ['2.6', '2.7', '3.1', head, jruby, truffleruby]

So, should we replace 3.1 with 3.0? Why should we remove 3.0 or should we only add 3.1? 🤔

@LeFnord
Copy link
Member

LeFnord commented Jul 26, 2022

@LeFnord

avoid calling of SimpleCov stuff

What do you mean by this? 🤔

sorry … mean it here spec_helper.rb

and you can also change ruby-version: ['2.6', '2.7', '3.0', head, jruby, truffleruby] into ruby-version: ['2.6', '2.7', '3.1', head, jruby, truffleruby]

So, should we replace 3.1 with 3.0? Why should we remove 3.0 or should we only add 3.1? 🤔

replace

@splattael splattael force-pushed the expose-argument-error branch from dbe0dc0 to 94c235e Compare July 26, 2022 22:19
@splattael
Copy link
Contributor Author

sorry … mean it here spec_helper.rb

I still don't follow. Should I delete SimpleCov? 🤔

replace

OK, done but honestly I don't understand why we stop testing on Ruby 3.0 🤷 It is still being maintained.

@LeFnord
Copy link
Member

LeFnord commented Jul 26, 2022

sorry my fault … yes you are right 3.0 is maintained … so re-add 3.0

and comment out the SimpleCov stuff here spec_helper.rb so the checks will be green and I can mörge it

@splattael splattael force-pushed the expose-argument-error branch from 94c235e to c3125cc Compare July 27, 2022 05:59
@splattael
Copy link
Contributor Author

sorry my fault … yes you are right 3.0 is maintained … so re-add 3.0

No worries, done 👍

and comment out the SimpleCov stuff here spec_helper.rb so the checks will be green and I can mörge it

Oh now I saw it fails on Ruby >= 3.1 due to simplecov-ruby/simplecov#1003 and thus cancels all other runs 🤦

I've disabled SimpleCov on Ruby >= 3.1 then so checks should pass soon 🤞

@LeFnord LeFnord merged commit 47eebba into ruby-grape:master Jul 27, 2022
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