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

Replace GPL'd colorize gem with MIT-licensed rainbow #408

Merged
merged 3 commits into from
Sep 1, 2016
Merged

Replace GPL'd colorize gem with MIT-licensed rainbow #408

merged 3 commits into from
Sep 1, 2016

Conversation

jamesc
Copy link
Contributor

@jamesc jamesc commented Aug 31, 2016

colorize is licensed under GPL-2 which conflicts with the MIT license of
github_changelog_generator. This changes all usage of colorize to
rainbow which does have a compatible license (MIT)

colorize is licensed under GPL-2 which conflicts with the MIT license of
github_changelog_generator.  This changes all usage of colorize to
rainbow which does have a compatible license (MIT)
@olleolleolle
Copy link
Collaborator

olleolleolle commented Sep 1, 2016

Hello James!

Good choice of library.

I wonder: we're coloring strings that pass through our log function in the Helper namespace. Idea: Can we ditch all spread-out coloring of these strings?

@olleolleolle
Copy link
Collaborator

@jamesc Hi! I made my comment into a PR-onto-your-PR.

@olleolleolle
Copy link
Collaborator

@jamesc Oh, I made a change, again.

  - The central logging method decides the color of the output
  - pass Rubocop complaints
  - do not include all the rainbow presenter methods directly in the
    String class
@@ -27,6 +27,9 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency "github_api", ">= 0.12"
spec.add_runtime_dependency "rainbow", ">= 2.1"

# rack 2.0 does not work on Ruby < 2.2. Lock it down here
spec.add_runtime_dependency "rack", "~> 1.2"
Copy link
Collaborator

@olleolleolle olleolleolle Sep 1, 2016

Choose a reason for hiding this comment

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

This is contentious: if people are using this library in their application, we're holding the rack back.

More to the point: people are using it in their applications, and I want us to find a way to make Ruby versions < 2.2 have a rack which is installable for them, while keeping users with 2.2+ happy.

@jamesc
Copy link
Contributor Author

jamesc commented Sep 1, 2016

Yeah, I backed out that change. It doesn't fix the travis builds anyway. I'll look at how we might be able to fix the builds in a separate branch.

@olleolleolle
Copy link
Collaborator

olleolleolle commented Sep 1, 2016

@jamesc Cool, thanks for your hard effort!

This change fixes a hard problem, and we got the logging mechanism placed more centrally, by only using its formatter. And we got rid of the String extension, which the colorize gem also had.

To make the PR really stand out in the changelog, do you want to rephrase the title of it? Since we will be making a CHANGELOG.md out of this, I want this PR to be called something clearer (for understanding at a glance). Examples include:

  • Use rainbow instead of colorize
  • Replace GPL'd colorize gem with MIT-licensed rainbow

@jamesc jamesc changed the title Change to use rainbow instead of colorize Replace GPL'd colorize gem with MIT-licensed rainbow Sep 1, 2016
@olleolleolle olleolleolle self-assigned this Sep 1, 2016
@olleolleolle olleolleolle merged commit 96491a2 into github-changelog-generator:master Sep 1, 2016
@olleolleolle
Copy link
Collaborator

Again, thanks a lot for doing this, @jamesc! 🎉

@olleolleolle olleolleolle removed their assignment Sep 1, 2016
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