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.

Sign up for GitHub

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

Jump to bottom

Fix Ruby 2.4 warnings because of Fixnum #62

Merged
merged 1 commit into from
Apr 29, 2017
Merged

Fix Ruby 2.4 warnings because of Fixnum #62

merged 1 commit into from
Apr 29, 2017

Conversation

JuanitoFatas
Copy link
Contributor

This PR fixes this warning:

/Users/juanito-fatas/.gem/ruby/2.4.0/gems/flag_shih_tzu-0.3.16/lib/flag_shih_tzu.rb:321: warning: constant ::Fixnum is deprecated

Ruby 2.4 unified Fixnum and Bignum into Integer.

https://www.ruby-lang.org/en/news/2016/12/25/ruby-2-4-0-released/

houndci-bot
houndci-bot reviewed Mar 10, 2017
View reviewed changes
lib/flag_shih_tzu.rb Outdated
@@ -318,7 +318,7 @@ def parse_flag_options(*args)
else
options.
keys.
select { |key| !key.is_a?(Fixnum) }.
select { |key| RUBY_VERSION >= "2.4" !key.is_a?(Integer) : !key.is_a?(Fixnum) }.
Copy link
Collaborator

Choose a reason for hiding this comment

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

unexpected token tBANG
unexpected token tCOLON

houndci-bot
houndci-bot reviewed Mar 10, 2017
View reviewed changes
lib/flag_shih_tzu.rb Outdated
@@ -318,7 +318,7 @@ def parse_flag_options(*args)
else
options.
keys.
select { |key| !key.is_a?(Fixnum) }.
select { |key| RUBY_VERSION >= "2.4" ? !key.is_a?(Integer) : !key.is_a?(Fixnum) }.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use 2 (not 0) spaces for indenting an expression spanning multiple lines.
Line is too long. [104/80]
Use Integer instead of Fixnum.

@coveralls
Copy link

coveralls commented Mar 10, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 53.817% when pulling 29c6823 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

coveralls commented Mar 10, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

coveralls commented Mar 10, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.817% when pulling c486e52 on JuanitoFatas:jf/fix-ruby24-warning into a09a04a on pboling:master.

@pboling
Copy link
Owner

pboling commented Mar 14, 2017

I need to get the build fixed so I understand what this will do to backwards compatibility with other Rubies.

@deivid-rodriguez
Copy link

@pboling Any news?

@pboling
Copy link
Owner

pboling commented Apr 20, 2017

I did work on fixing the build for a few hours, but had to give up to try again another day. It is on my list of things to get done soon. I do not actively use this gem in any projects at the moment, so PRs appreciated!

@deivid-rodriguez
Copy link

Thanks @pboling! So what's the status here? Is the build currently broken and you prefer to get that sorted out first, or is it this PR breaking the build somehow? I guess the former?

@pboling
Copy link
Owner

pboling commented Apr 26, 2017 •
edited
Loading

Yeah, I'd rather not release with a broken build. It has been a little broken for a while now. Just needs maintenance. Nothing to do with any PR AFAIK.

@deivid-rodriguez
Copy link

Yeah, sounds sensible. I'll look into helping out!

@pboling
Copy link
Owner

pboling commented Apr 29, 2017 •
edited
Loading

@JuanitoFatas Please rebase this PR on latest master, the build has been fixed! Want to push a release ASAP! I did it. :) I did not include this in the release yet, because I still want to see how this affects older Rubies.

@pboling pboling merged commit c486e52 into pboling:master Apr 29, 2017
@JuanitoFatas JuanitoFatas deleted the jf/fix-ruby24-warning branch April 30, 2017 12:32
@JuanitoFatas
Copy link
Contributor Author

Thanks so much for the merged!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JuanitoFatas @coveralls @pboling @deivid-rodriguez @houndci-bot