-
-
Notifications
You must be signed in to change notification settings - Fork 519
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
Luhn with generator #559
Luhn with generator #559
Conversation
Cases and template seem to be very simple, and may be used in documenting this process.
Failing due to Ruby version not having |
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.
Looks good, it generates a correct looking test file which is the main thing.
exercises/luhn/example.tt
Outdated
class LuhnTest < Minitest::Test<% test_cases.each do |test_case| %> | ||
def <%= test_case.name %> | ||
<%= test_case.skipped %> | ||
<%= test_case.assertion %> <%= test_case.work_load %> |
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.
test_case.workload
should include the assertion.
So this line can be simplified to: <%= test_case.workload %>
lib/luhn_cases.rb
Outdated
index.zero? ? '# skip' : 'skip' | ||
end | ||
|
||
def assertion |
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 can be made private as it is only needed by workload
lib/luhn_cases.rb
Outdated
@@ -6,13 +6,15 @@ def name | |||
end | |||
|
|||
def work_load | |||
%Q(Luhn.valid?("#{input}")) | |||
%Q(#{assertion} Luhn.valid?("#{input}")) |
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.
"#{input}"
This will work, but it's a good habit to use input.inspect
which will handle input that contains quotes.
Nice usage of %Q
though. 👍
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.
Good idea!
lib/luhn_cases.rb
Outdated
'test_%s' % description.tr('- ', '__') | ||
end | ||
|
||
def work_load |
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 should be workload
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.
I've updated the documentation to help clarify this: #560
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.
Is there a style guide somewhere (other than the README)? Because some of the exercises use work_load
and others use workload
. In fact, as long as it is the same in both example.tt
and <exercise>_cases.rb
it should work fine.
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.
No, just the README.
I've made a PR to rename the old occurrences to try and clear this up: #561
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.
Looks great. Nice work @hilary
Thanks for doing this @hilary ❤️ It will be some non-deterministic amount of time until this change is visible as it requires Katrina to do a release to the live site, but that will probably happen sometime in the next day or two. |
Description
Added Generator for Luhn for new Luhn canonical data
Motivation and Context
The Luhn exercise had some weaknesses (exercism/problem-specifications#474) which have now been addressed via canonical data (exercism/problem-specifications#491).
How Has This Been Tested?
We ran
rake test
Types of changes
References and Closures
Checklist: