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

Fix documentation (validation option, slider example) #133

Merged
merged 1 commit into from
Mar 22, 2020

Conversation

2called-chaos
Copy link
Contributor

@2called-chaos 2called-chaos commented Mar 22, 2020

Describe the change

Fixes documentation in Readme

Why are we doing this?

For Sparta!

Benefits

Functioning examples

Drawbacks

No puzzle for developers

Requirements

Put an X between brackets on each line if you have done the item:
[] Tests written & passing locally? (there is in fact no test using the validation option opposed to the #validate method. should I modify one of the existing or add a new test?)
[X] Code style checked?
[X] Rebased with master branch?
[X] Documentation updated?


As per options.fetch(:validation)

The other example lead to incomplete format specifier; use %% (double %) instead

@coveralls
Copy link

coveralls commented Mar 22, 2020

Coverage Status

Coverage decreased (-0.2%) to 96.991% when pulling 5c327b9 on 2called-chaos:master into 65187af on piotrmurach:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 97.025% when pulling fed650a on 2called-chaos:master into 65187af on piotrmurach:master.

@piotrmurach
Copy link
Owner

Hi Sven,

Thanks for this PR! I think this is a bug that you found here rather than document fix! The DSL method is called validate and the key should also match that and be called :validate. Could you please change this PR to only include the slider readme fix. If you have time and want to help I would appreciate fixing the option with a test confirming the fix.

@piotrmurach piotrmurach merged commit b941ff7 into piotrmurach:master Mar 22, 2020
@piotrmurach
Copy link
Owner

Thanks ❤️

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.

3 participants