-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
ternary_value_flag feature without tests #56
base: master
Are you sure you want to change the base?
ternary_value_flag feature without tests #56
Conversation
1 similar comment
2152d95
to
cad429c
Compare
2 similar comments
cad429c
to
def53dc
Compare
1 similar comment
def53dc
to
6a6e5e4
Compare
ef9b61b
to
fe9a2f4
Compare
1 similar comment
8cd92ea
to
d8d120f
Compare
6 similar comments
1 similar comment
1 similar comment
d8d120f
to
d655c4f
Compare
collect_flags(*flags_to_collect) do |memo, flag| | ||
memo << "not_#{flag}".to_sym unless truthy_and_chosen.include?(flag) | ||
memo << "nil_#{flag}".to_sym if self.send(flag) == false |
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.
Redundant self detected.
truthy_and_chosen.concat( | ||
untruthy_and_unchosen = ( | ||
collect_flags(*flags_to_collect) do |memo, flag| | ||
memo << "not_#{flag}".to_sym if self.send(flag) == false |
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.
Redundant self detected.
@@ -18,6 +21,7 @@ def self.included(base) | |||
class IncorrectFlagColumnException < Exception; end | |||
class NoSuchFlagQueryModeException < Exception; end | |||
class NoSuchFlagException < Exception; end | |||
class InvalidValueForFlagException < Exception; end |
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.
Inherit from RuntimeError instead of Exception.
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.
Yeah, all of these classes should not be inheriting from Exception
. 💯 Needs to be fixed.
@shivang2056yadav - wow! This is really cool! I need to spend more time looking over it. |
d655c4f
to
4b19eef
Compare
Needs to be rebased on latest master, which now has a passing build on Travis. |
No description provided.