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

**nil no keyword mark method definitions #796

Merged
merged 1 commit into from
Oct 11, 2020

Conversation

moofkit
Copy link
Contributor

@moofkit moofkit commented Oct 10, 2020

Solving #745

**nil is allowed in method definitions to explicitly mark
that the method accepts no keywords. Calling such a method with keywords
will result in an ArgumentError. Feature #14183

PS I'm not sure that it is correct place for spec...

@andrykonchin
Copy link
Member

Looks good 👍 .

Wondering why this new spec is under guard ruby_version_is "2.7"...'3.0' instead of ruby_version_is "2.7"...''. Is there any reason to declare the upper version?

ruby_version_is "2.7"...'3.0' do

@moofkit
Copy link
Contributor Author

moofkit commented Oct 11, 2020

@andrykonchin thank you for review!
I have found some a little details about this feature here https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/
Yeah, I think you are right and it is should be under ruby_version_is "2.7"...'' guard.

But there are no such guard yet in this spec. Should I add it?

@moofkit
Copy link
Contributor Author

moofkit commented Oct 11, 2020

Ahh, I see https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#version-guards

ruby_version_is "2.7" do
  # Specs for RUBY_VERSION >= 2.7
end

@moofkit
Copy link
Contributor Author

moofkit commented Oct 11, 2020

@andrykonchin moved spec under ruby_version_is "2.7" guard.

After several moving and reading specs context I've decide to leave it under separated guard block. Commits was squashed for clear history

@andrykonchin andrykonchin merged commit 7a5bd53 into ruby:master Oct 11, 2020
@andrykonchin
Copy link
Member

Great. Thank you for the new specs!

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.

2 participants