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 customization of placeholder line; #24

Merged
merged 15 commits into from
Mar 29, 2017

Conversation

Antondomashnev
Copy link
Collaborator

Hi @dblock this PR addresses the #11 issue. I've started with configuration object which could eventually help to resolve multiple issues easier. Let me know what you think about implementation.

@dblock
Copy link
Owner

dblock commented Mar 27, 2017

I like where it's going, but maybe it's overly complicated?

Is there any way to have more than one of these configurations? I don't think so, so I would just create a global configuration object ala https://github.com/slack-ruby/slack-ruby-client/blob/master/lib/slack/config.rb and call it a day. Then we can do DangerChangelog.configure do |config|. This would avoid passing it around as parameters to start.

@Antondomashnev
Copy link
Collaborator Author

@dblock was one of the ideas as well, I think you're right would be clearer and we don't need multiple configurations. I'll update the PR with that approach.

@Antondomashnev
Copy link
Collaborator Author

@dblock check out the updated PR, it looks much better now, thanks for your suggestion - every day we learn 😄

@Antondomashnev
Copy link
Collaborator Author

@dblock the Travis CI is failing and it seems because of the rubocop's new version. Is there a reason to keep the Gemfile.lock in gitignore?

@dblock
Copy link
Owner

dblock commented Mar 29, 2017

Since Gemfile.lock isn't committed rubocop should be locked at a specific version. I usually run rubocop -a ; rubocop --auto-gen-config.

@dblock
Copy link
Owner

dblock commented Mar 29, 2017

Perfect, merging.

@dblock dblock merged commit b0a9677 into dblock:master Mar 29, 2017
@Antondomashnev Antondomashnev deleted the customize_placeholder_line branch March 29, 2017 06:12
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.

2 participants