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 configuration options #6

Merged
merged 1 commit into from Apr 4, 2017

Conversation

@supremebeing7
Copy link
Contributor

commented Apr 1, 2017

Fixes #2

There's one piece that I have not done yet because I wanted to get your input, @mmozuras:
renaming the FiftyCharSummary and SeventyTwoCharLimit classes. Since those are now configurable, it seems odd to have a specific number as a part of the class name. Would you agree?

Also, if you like I can squash the commits down to one.

Let me know what you think.

@mmozuras

This comment has been minimized.

Copy link
Owner

commented Apr 2, 2017

Since those are now configurable, it seems odd to have a specific number as a part of the class name. Would you agree?

@supremebeing7 completely agree!

Also, if you like I can squash the commits down to one.

Yes, that would be preferable 😄

@config.summary_character_limit_number.to_i
end

def error_message

This comment has been minimized.

Copy link
@mmozuras

mmozuras Apr 2, 2017

Owner

@supremebeing7 why did you change it from a constant to a method?

This comment has been minimized.

Copy link
@supremebeing7

supremebeing7 Apr 3, 2017

Author Contributor

I had to do that for the character count rules in order to interpolate the number of characters. The rest I changed for consistency.

This comment has been minimized.

Copy link
@mmozuras

mmozuras Apr 3, 2017

Owner

@supremebeing7 👌

Do these and I'm ready to merge! 😄

This comment has been minimized.

Copy link
@supremebeing7

supremebeing7 Apr 3, 2017

Author Contributor

Done 🎉

- Add config options
- Add sample yml file
- Add configuration to README
- Rename character limit rules to correspond with configurability
@supremebeing7 supremebeing7 force-pushed the supremebeing7:config branch from 9e56412 to 3ca9cd4 Apr 3, 2017
@mmozuras mmozuras merged commit 428922d into mmozuras:master Apr 4, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mmozuras

This comment has been minimized.

Copy link
Owner

commented Apr 4, 2017

@supremebeing7 perfect 👍

@mmozuras

This comment has been minimized.

Copy link
Owner

commented Apr 4, 2017

Released as v0.2.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.