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

Add new validation feature for settings. #49

Merged
merged 5 commits into from
Jun 23, 2020
Merged

Add new validation feature for settings. #49

merged 5 commits into from
Jun 23, 2020

Conversation

jwoertink
Copy link
Member

Fixes #41

This adds in a new feature that lets us add some validation beyond just the existence of a value to our settings. When you add the validation key, you specify a symbol of the method that will run the validation. This will take an argument of the type you specified in your setting.

Inside that method, you can run any sort of validation you wish. If that validation fails, you can call settings.invalid() with a message to be raised. This will raise an InvalidSettingFormatError with your message.

@jwoertink jwoertink requested a review from paulcsmith June 19, 2020 00:41
Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

This is a really nice implementation. I thought it'd have to be more complex but you did it in a super simple and powerful way. I dig it.

Just one comment on making it clearer that it is raising, and where the method comes from (Habitat)

spec/habitat_spec.cr Outdated Show resolved Hide resolved
@jwoertink
Copy link
Member Author

@paulcsmith If you're cool with this, I'll merge it in. 👍

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

Looks great :D

@jwoertink jwoertink merged commit adfd999 into master Jun 23, 2020
@jwoertink jwoertink deleted the features/41 branch June 23, 2020 19:59
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.

Feature: A pattern matching option
2 participants