Skip to content

Fix calling without argument warnings#31

Merged
henrik merged 1 commit into
barsoom:masterfrom
JuanitoFatas:fix-ruby27-warnings
Jan 7, 2020
Merged

Fix calling without argument warnings#31
henrik merged 1 commit into
barsoom:masterfrom
JuanitoFatas:fix-ruby27-warnings

Conversation

@JuanitoFatas
Copy link
Copy Markdown
Contributor

Hello, this Pull Request has 2 changes:

  • Remove public since it does not generate "private attribute?" warnings and this also fixes for following warning

    warning: calling public without arguments inside a method may not
    have the intended effect
    
  • Check if names are not empty to fix following warning:

    warning: calling private without arguments inside a method may not
    have the intended effect
    

These calling without arguments warnings introduced by ruby/ruby@2993b24 from ruby bug tracker #13249.

I‘m not a fan of adding this if, but \_(ツ)_/.

Open to better suggestion, thanks!

Remove public since it does not generate "private attribute?" warnings and for following warning

  warning: calling public without arguments inside a method may not have the intended effect

Check if names are not empty to fix following warning:

  warning: calling private without arguments inside a method may not have the intended effect

This warning was introduced by ruby/ruby@2993b24 from issue:
https://bugs.ruby-lang.org/issues/13249
@henrik
Copy link
Copy Markdown
Member

henrik commented Jan 7, 2020

Thank you! We were about to look into this but this saves us the trouble :)

I tried running the tests with this change on Ruby 2.5.1 and 2.6.5, and it didn't show a "private attribute?" warning (even with RUBYOPT=-w rake), so it seems it's OK for at least those two as well.

@henrik henrik merged commit cf1b134 into barsoom:master Jan 7, 2020
@henrik
Copy link
Copy Markdown
Member

henrik commented Jan 7, 2020

We changed it to a guard :) 1305e6c

@JuanitoFatas JuanitoFatas deleted the fix-ruby27-warnings branch January 8, 2020 18:02
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 this pull request may close these issues.

2 participants