davetron5000 / optparse-plus

Start your command line scripts off right in Ruby
http://davetron5000.github.com/optparse-plus
Apache License 2.0
521 stars 54 forks source link

Issues with Ruby 2.4 #110

Closed matthutchinson closed 6 years ago

matthutchinson commented 7 years ago

I maintain the lolcommits gem, (which has been using methadone for some years now). I'm working on making it Ruby 2.4.0 friendly. Among other issues I found that methadone was blowing up with this error when calling go!:

NoMethodError: undefined method `level' for nil:NilClass
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/cli_logger.rb:102:in `level='
  /Users/matt/.rbenv/versions/2.4.0/lib/ruby/2.4.0/logger.rb:380:in `initialize'
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/cli_logger.rb:85:in `initialize'
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/cli_logging.rb:37:in `new'
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/cli_logging.rb:37:in `logger'
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/main.rb:405:in `rescue in call_main
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/main.rb:395:in `call_main'
  /u/code/lolcommits/vendor/bundle/gems/methadone-1.9.4/lib/methadone/main.rb:167:in `go!'

Digging into this.. it turns out that Logger in Ruby 2.4.0 has a new initializemethod, that takes new optional args and calls to set the logger level (defaulting to DEBUG). Here's the new method signature..

# lib/ruby/2.4.0/logger.rb
def initialize(logdev, shift_age = 0, shift_size = 1048576, level: DEBUG,
               progname: nil, formatter: nil, datetime_format: nil,      
               shift_period_suffix: '%Y%m%d')                            

This would be fine, except that Methadone::CLILogger overrides the level= method and expects that a @stderr_logger variable is set. The fix for this I'd propose, would be to set this instance variable first, before calling super(log_device). That way, it will always be present, when the Ruby Logger calls to set level= in it's own initializer.

I was preparing a fork/PR for this change when I noticed these:


This article explains whats new in Ruby 2.4 (including the above changes I mentioned).

I'll issue a PR in a few secs, hopefully it will pass Travis for all Rubies...

davetron5000 commented 7 years ago

Yeah, on Travis it seemed like it couldn't install the json gem. Not sure if that's just Travis or if there are deeper issues. Please LMK if you find problems

matthutchinson commented 7 years ago

Alll green on Travis ! 💚

(apologies for the whitespace changes in my PR, my Vim is set to truncate leading whitespace)

davetron5000 commented 6 years ago

This was fixed by #111