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

Cleanup #220

Merged
merged 14 commits into from Mar 27, 2015
Merged

Cleanup #220

merged 14 commits into from Mar 27, 2015

Conversation

tuexss
Copy link
Contributor

@tuexss tuexss commented Mar 26, 2015

I tried to add a bit of consistency and remove unused or unnecessary lines of code. I hope this helps a bit.

@@ -139,7 +139,7 @@ def fetch_merged_at_pull_requests
}

if @options[:verbose]
puts 'Fetching merged dates... Done!'
puts "Fetching merged dates: Done!"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for cleanup! I think there is should be single quotes, since you don't use any parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would've gone with http://viget.com/extend/just-use-double-quoted-ruby-strings and always use double quotes. But if you prefer mixed style, I can revert this part :)

Copy link
Member

Choose a reason for hiding this comment

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

@tuexss it really makes sense. I just new in ruby, and consider, if RubyMine and rubocop pronouncedly highlight this - it should be like that "a priori". So, I thinking to move to double quote and revert this "fancy" PR :)

.. and add to rubocom.yml.

Style/StringLiterals:
  EnforcedStyle: double_quotes

Thanks, for point it out!

@skywinder
Copy link
Member

Wow! @tuexss it seems you are deeply inspect this code and fix bunch of different typos and logic issues! Now it looks much healthy! 💎
Thanks a lot! 👍

skywinder added a commit that referenced this pull request Mar 27, 2015
@skywinder skywinder merged commit 8de0b33 into github-changelog-generator:master Mar 27, 2015
@tuexss
Copy link
Contributor Author

tuexss commented Mar 27, 2015

Yes, i'm also quite new in ruby. Thanks for the merge!

skywinder added a commit that referenced this pull request Apr 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants