Skip to content

Add check_default_type! #569

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

Merged
merged 2 commits into from
Jul 6, 2017
Merged

Add check_default_type! #569

merged 2 commits into from
Jul 6, 2017

Conversation

rafaelfranca
Copy link
Member

It check if the default value of an option matches the defined type.

This removes the warning on usage since there is no much what the command users could do about it and give the command authors the possibility to check for programming errors.

lib/thor/base.rb Outdated
@@ -151,6 +151,21 @@ def check_unknown_options?(config) #:nodoc:
!!check_unknown_options
end

# If you want to raise an error when the default value of an option does not match
# the type call check_default_type!
# This is disabled by default.
Copy link
Member

Choose a reason for hiding this comment

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

+ "for compatibility"

CHANGELOG.md Outdated
@@ -1,4 +1,7 @@
## 0.20.0
* Add `check_default_type!` to check if the deafault value of an option matches the defined type.
It removes the warning on usage and give the command authors the posibility to check for programing erros.
Copy link
Member

Choose a reason for hiding this comment

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

Few typos in here: default, possibility, errors

@@ -4,13 +4,14 @@ class Option < Argument #:nodoc:

VALID_TYPES = [:boolean, :numeric, :hash, :array, :string]

def initialize(name, options = {})
def initialize(name, options = {}, check_default_type = false)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be passed in options?

Choose a reason for hiding this comment

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

Sounds like options is where it belongs, yeah.

@@ -583,6 +583,24 @@ def unknown(*args)
expect(klass.start(%w(unknown foo --bar baz))).to eq(%w(foo))
end

it "does not checks the default type" do

Choose a reason for hiding this comment

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

it "does not check the default type when check_default_type! is not called" do

@@ -4,13 +4,14 @@ class Option < Argument #:nodoc:

VALID_TYPES = [:boolean, :numeric, :hash, :array, :string]

def initialize(name, options = {})
def initialize(name, options = {}, check_default_type = false)

Choose a reason for hiding this comment

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

Sounds like options is where it belongs, yeah.

@rafaelfranca rafaelfranca force-pushed the check_default_type branch 3 times, most recently from bc27d59 to d015cdc Compare July 6, 2017 18:58
@rafaelfranca
Copy link
Member Author

Fixed all the points

CHANGELOG.md Outdated
@@ -1,4 +1,7 @@
## 0.20.0
* Add `check_default_type!` to check if the default value of an option matches the defined type.
It removes the warning on usage and give the command authors the possibility to check for programming errors.

Choose a reason for hiding this comment

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

... and gives the command ...

Copy link

@andremedeiros andremedeiros left a comment

Choose a reason for hiding this comment

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

Minor typo but 👍

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
It check if the default value of an option matches the defined type.

This removes the warning on usage since there is no much what the command
users could do about it and give the command authors the possibility to check
for programming errors.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@rafaelfranca rafaelfranca merged commit df5ba2b into master Jul 6, 2017
@rafaelfranca rafaelfranca deleted the check_default_type branch July 6, 2017 20:34
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.

None yet

3 participants