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

Avoid Ruby warnings #319

Conversation

olleolleolle
Copy link
Contributor

This PR was made when working on removing warnings in another project, and there were a few warnings output from this project.

To avoid warnings, this PR:

  • removes unused variables
  • adds parentheses when Ruby complains
  • tries to do as little as possible while still avoiding the warning

@@ -21,7 +21,7 @@ def initialize(message = $!)
end

def backtrace
if @response_message && @response_message.respond_to?(:backtrace)
if defined?(@response_message) && @response_message && @response_message.respond_to?(:backtrace)

Choose a reason for hiding this comment

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

Line is too long. [104/80]

@@ -35,7 +35,7 @@ def list(*args)
#
# @api public
def get(*args)
params = arguments(args, required: [:owner, :repo, :id]).params
arguments(args, required: [:owner, :repo, :id]).params

Choose a reason for hiding this comment

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

Use %i or %I for an array of symbols.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@piotrmurach If I fix this according to the RuboCop rules now in place, the code will be invalid Ruby for the MRI 1.9.3 that's in the Travis test matrix.

@@ -49,7 +49,7 @@ def render(*args)
#
def render_raw(*args)
params = arguments(args).params
mime_type, params['data'] = params['mime'], args.shift
_mime_type, params['data'] = params['mime'], args.shift

Choose a reason for hiding this comment

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

Do not use parallel assignment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

if
defined?(@response_message) &&
@response_message &&
@response_message.respond_to?(:backtrace)

Choose a reason for hiding this comment

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

Align the operands of a condition in an if statement spanning multiple lines.

if @response_message && @response_message.respond_to?(:backtrace)
if
defined?(@response_message) &&
@response_message &&

Choose a reason for hiding this comment

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

Align the operands of a condition in an if statement spanning multiple lines.

@@ -21,7 +21,10 @@ def initialize(message = $!)
end

def backtrace
if @response_message && @response_message.respond_to?(:backtrace)
if
defined?(@response_message) &&

Choose a reason for hiding this comment

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

Place the condition on the same line as if.

if
defined?(@response_message) &&
@response_message &&
@response_message.respond_to?(:backtrace)

Choose a reason for hiding this comment

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

Align the operands of a condition in an if statement spanning multiple lines.

if @response_message && @response_message.respond_to?(:backtrace)
if
defined?(@response_message) &&
@response_message &&

Choose a reason for hiding this comment

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

Align the operands of a condition in an if statement spanning multiple lines.

@@ -21,7 +21,10 @@ def initialize(message = $!)
end

def backtrace
if @response_message && @response_message.respond_to?(:backtrace)
if
defined?(@response_message) &&

Choose a reason for hiding this comment

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

Place the condition on the same line as if.

@coveralls
Copy link

coveralls commented Jul 11, 2017

Coverage Status

Coverage decreased (-0.0006%) to 96.55% when pulling d997a88 on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0006%) to 96.55% when pulling d997a88 on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

@coveralls
Copy link

coveralls commented Jul 11, 2017

Coverage Status

Coverage decreased (-0.009%) to 96.542% when pulling 4ed89fc on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

@coveralls
Copy link

coveralls commented Jul 11, 2017

Coverage Status

Coverage decreased (-0.04%) to 96.508% when pulling 1db295b on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

@olleolleolle olleolleolle force-pushed the fix/avoid-warnings-when-running-specs branch from 3a8d875 to 295fde5 Compare July 11, 2017 22:30
@coveralls
Copy link

coveralls commented Jul 11, 2017

Coverage Status

Coverage decreased (-0.0006%) to 96.55% when pulling 3a8d875 on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

@coveralls
Copy link

coveralls commented Jul 11, 2017

Coverage Status

Coverage decreased (-0.0006%) to 96.55% when pulling 295fde5 on olleolleolle:fix/avoid-warnings-when-running-specs into 4a9be11 on piotrmurach:master.

@piotrmurach piotrmurach merged commit d51f45a into piotrmurach:master Jul 14, 2017
@piotrmurach
Copy link
Owner

Thanks for cleaning things up! ❤️

@olleolleolle olleolleolle deleted the fix/avoid-warnings-when-running-specs branch July 14, 2017 21:06
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.

None yet

4 participants