Skip to content

Make KeywordArgs fail if unexpected keys are passed in #187

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

abevoelker
Copy link
Contributor

Wasn't sure where to put the KeywordArgs specs - let me know if it should go in spec/contracts_spec.rb or whatnot (or if I'm otherwise way off base)

@abevoelker
Copy link
Contributor Author

Hmm weird, specs passed with 2.2 on my machine. I'll look into the failures.

@abevoelker
Copy link
Contributor Author

Ah bundle exec rake was passing, but it seems that bundle exec rspec does fail on my machine as it runs more specs than rake...

@abevoelker abevoelker force-pushed the keywordargs_unexpected_keys branch from 61f67c9 to 31e2066 Compare July 23, 2015 00:12
@alex-fedorov
Copy link
Collaborator

Looks like you actually want to fix this spec. Because it tries to pass in unexpected argument and expects it to not fail. Seems like a buggy behavior to me.

@abevoelker
Copy link
Contributor Author

Okay I think I fixed it - let me know if I violated the intent of the spec or whatnot

options.all? do |key, contract|
Optional._valid?(hash, key, contract)
end
(hash.keys - options.keys == []) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use guard statement here instead - current code makes it hard to read. Example:

def valid?(hash)
  return false unless hash.keys - options.keys == []      # <--- this is a guard statement
  # .. do the main job here ..
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah for sure, should've asked about that

@alex-fedorov
Copy link
Collaborator

Other than that, looks good.

@abevoelker abevoelker force-pushed the keywordargs_unexpected_keys branch from 31e2066 to 1960743 Compare July 23, 2015 00:26
@alex-fedorov
Copy link
Collaborator

@abevoelker Thanks!

alex-fedorov added a commit that referenced this pull request Jul 23, 2015
Make KeywordArgs fail if unexpected keys are passed in
@alex-fedorov alex-fedorov merged commit a6df773 into egonSchiele:master Jul 23, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants