-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Stop yielding skip value #2341
Stop yielding skip value #2341
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, but I fail to see how it can break someone 🤔 How was this a problem for your use-case?
At least we should add a paragraph to UPGRADING.
) | ||
end | ||
end | ||
|
||
context 'when missing optional value' do | ||
let(:params) { [Grape::DSL::Parameters::EmptyOptionalValue, 10] } | ||
|
||
it 'marks params with skipped values' do | ||
it 'doest not yield skipped values' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: does not
I'm not sure its a breaking change. There's never been a validation done against a skip value. It's was always returned by base class when validating. I figured that why yield these values when it's actually returned automatically. |
I think you're right. I am going to YOLO it. |
This PR will stop yielding
skip
value since iterators are already ignoringskip
values.Resolves #2329