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

Shut up UselessMethodDefinition cop #1014

Merged
merged 2 commits into from Aug 23, 2020

Conversation

pirj
Copy link
Member

@pirj pirj commented Aug 20, 2020

This fixes edge-rubocop CI job

Caused by this bug rubocop/rubocop#8561


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • [-] Added tests.
  • [-] Updated documentation.
  • [-] Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

@pirj pirj self-assigned this Aug 20, 2020
@pirj pirj added this to the 2.0 milestone Aug 20, 2020
@pirj pirj removed this from the 2.0 milestone Aug 21, 2020
@@ -9,11 +9,11 @@ module ExpectOffense

DEFAULT_FILENAME = 'example_spec.rb'

def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs)
def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs) # rubocop:disable Lint/UselessMethodDefinition
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because we usually test with an older version of RuboCop, we need to disable Lint/RedundantCopDisableDirective too.

Maybe a "block" disable will work better than the trailing line comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha, fair enough.

We could wait until this cop is released like we did here, but it's making the edge build red for the time being.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess a green build is better than a few redundant disable directives here and there. We can address them from time to time.

Copy link
Member

Choose a reason for hiding this comment

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

But is this a false positive in UselessMethodDefinition as it changes the default value for an argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, yes. Filed rubocop/rubocop#8561

@pirj pirj force-pushed the shut-up-useless-useless-method-cop branch from d267a61 to cb85669 Compare August 23, 2020 13:44
@bquorning bquorning merged commit 5916bd1 into master Aug 23, 2020
@bquorning bquorning deleted the shut-up-useless-useless-method-cop branch August 23, 2020 19:20
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

3 participants