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

Specify colorize version dependency #151

Closed
Confusion opened this issue Jun 11, 2014 · 12 comments · Fixed by #161
Closed

Specify colorize version dependency #151

Confusion opened this issue Jun 11, 2014 · 12 comments · Fixed by #161

Comments

@Confusion
Copy link

Current sshkit depends on colorize 0.6.0+, because it expects it has defined the Color.bold method, which only happens if colorize defines the bold mode, which wasn't the case in versions before 0.6.0.

If you are using an older colorize, you will get

Exception while executing on host 127.0.0.1: undefined method `bold' for Color:Module
$PROJECT_ROOT/vendor/bundle/ruby/2.0.0/gems/sshkit-1.5.1/lib/sshkit/formatters/pretty.rb:22:in `write_command'
@mattjmorrison
Copy link

👍 as of Colorize 0.7.5 the API changed and String::COLORS no longer exists.

@kalys
Copy link

kalys commented Dec 16, 2014

@mattjmorrison I cannot work with capistrano because of this.

@baires
Copy link

baires commented Dec 16, 2014

Same here

@leehambley
Copy link
Member

If someone can report a last known good version, I'll lock the gem dependency. Similar issue on Capistrano reported ~20 mins ago (now closed) capistrano/capistrano#1228

@kalys
Copy link

kalys commented Dec 16, 2014

0.7.4 is ok.

@sponomarev
Copy link
Contributor

@leehambley 0.7.3 is good enough

@leehambley
Copy link
Member

Seems to have been changed in fazibear/colorize@7cba4a1#diff-f8d8a85a7e89db8ca4c5dbfd9944ccc8R25, man their CHANGELOG is a mess...

Workaround for now, you can lock that dependency additional next to SSHKit, a Real fix for SSHKit is tricky as we try really hard not to lock dependencies on Rake, and other "likely to be required from other gems" deps, so that if we are slow in updating our locked gems, then we don't break other projects, I think the correct fix for this might be to switch on the Colorize::Version and try both APIs to avoid locking one specific range.

I can't believe they've completely changed their API in a "feature" release, seems a bit overkill and like what I might interpret to be a violation of my understanding of semver.

If someone can send a PR, I'd be very appreciative, otherwise I'll likely get to fixing it before the 20th, but not before.

@sponomarev
Copy link
Contributor

@leehambley Is it nice to change sshkit to accept new colorize API?

@leehambley
Copy link
Member

The problem is that I really hate when tools which aren't part of your apps dependencies (like Cap, and SSHKit, by proxy) are forcing hard gem dependencies, what if you have some logger in your app, which locks colorize ~> 0.6.0, and you want to use Cap as well… then you have to choose between dev time and production time dependencies because of the way Bundler works, and I really try hard to avoid forcing that on people.

If a short term solution is to make a harder gem requirement on a known good older version of colorize, fair enough, it might stop disappointing new users who have a clean gemset.

But in the end, I'd like to support both APIs, and not force users into using one, or the other version of the colorize gem, and by proxy it's API.

@fazibear
Copy link

Why you don'y use String.colors and String.modes methods. It's part of API. It's the same across all colorize versions. Besides it gives you just color and mode names.
#199

@leehambley
Copy link
Member

Seems like colorize don't bother to document their API, if you can send a PR to replace the one #198 I just merged, I'd be glad of it... I don't care how we get it fixed, as long as we do. 1.6.0 fixed the version dep on 0.6x as a stop-gap.

@fazibear
Copy link

This is from README:

String.colors - return array of all possible colors names
String.modes - return array of all possible modes

And this is PR that use this methods: #199

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 a pull request may close this issue.

7 participants