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

Fix build error using RuboCop 1.49+ #166

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Fix build error using RuboCop 1.49+ #166

merged 1 commit into from
Apr 25, 2023

Conversation

koic
Copy link
Contributor

@koic koic commented Apr 25, 2023

Fixes rubocop/rubocop#11817.

The test code is updated with the following command to resolve the issue:

$ bundle exec rubocop --require rubocop/cop/internal_affairs \
  --only InternalAffairs/RedundantDescribedClassAsSubject,Layout/EmptyLines -a spec

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake. It executes all tests and RuboCop for itself, and generates the documentation.

Fixes rubocop/rubocop#11817.

The test code is updated with the following command to resolve the issue:

```console
$ bundle exec rubocop --require rubocop/cop/internal_affairs \
  --only InternalAffairs/RedundantDescribedClassAsSubject,Layout/EmptyLines -a spec
```
@thomthom
Copy link
Member

Oh! Thank you for this PR!

This also answers another think I was curious about - running the InternalAffairs cops on our extension. (I'd just now noticed them in the RuboCop repo.) I'll see if I can hook them up to run with the rest of the checks.

Can I ask some follow up questions to better understand the changes?

I see the gist of the changes is adding , :config to describe and removing subject(:cop) { described_class.new(config) }.
Was this somehow masking the true config that should have been passed to the cops in the tests?
(I'm not super familiar with rspec, I've mostly used it in this project where I used it to more easily mesh with RuboCop. I'm more familiar with MiniTest.)

@thomthom thomthom merged commit 679d2e2 into SketchUp:main Apr 25, 2023
6 checks passed
@koic koic deleted the wip branch April 26, 2023 03:09
@koic
Copy link
Contributor Author

koic commented Apr 26, 2023

Simple configuration test cases are better replaced like the changes in this PR so that the following shared_context is enabled:
https://github.com/rubocop/rubocop/blob/v1.50.2/lib/rubocop/rspec/shared_contexts.rb#L64-L118

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.

Behaviour change between 1.48.1 and 1.49.0 - specs expect cop name in offense message
2 participants