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

Expose 'with default RSpec/Language config' to consumers #1377

Merged

Conversation

smcgivern
Copy link
Contributor

@smcgivern smcgivern commented Sep 8, 2022

This helper is essential for anyone writing their own RSpec cops based on RuboCop::Cop::RSpec::Base, because it now requires a minimal config to even run the cop.

The wordy file name is because the RuboCop gem already defines rubocop/rspec/shared_contexts, and we don't want to shadow that. This means that consumers of this library who want to write a spec for a cop based on RuboCop::Cop::RSpec::Base will need something like this in spec/spec_helper.rb:

require 'rubocop/rspec/shared_contexts/default_rspec_language_config_context'

RSpec.config do |config|
  # ...
  # Add metadata as appropriate
  config.include_context 'with default RSpec/Language config'
end

This was suggested in the PR that introduced this feature: https://github.com/rubocop/rubocop-rspec/pull/956/files#r515643915

We don't enforce using alias configuration, and custom cops can continue to use their own configuration until someone decides to follow the mainstream, and would like to contribute by extracting this somewhere to lib/.

fixes #1418

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).

@smcgivern
Copy link
Contributor Author

Updated documentation.

I'd like to do this, but I wasn't sure where. Maybe https://github.com/rubocop/rubocop-rspec/blob/master/docs/modules/ROOT/pages/upgrade_to_version_2.adoc?

@smcgivern
Copy link
Contributor Author

I tested this on a library we have that uses rubocop-rspec and it worked for me: https://gitlab.com/gitlab-org/ruby/gems/gitlab-styles/-/merge_requests/119#note_1092839680

@bquorning bquorning requested a review from pirj September 10, 2022 14:49
spec/spec_helper.rb Outdated Show resolved Hide resolved
@pirj
Copy link
Member

pirj commented Sep 12, 2022

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Thank you!
Appreciate if you could add some docs.

@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch 2 times, most recently from f7e2dc5 to ad674c8 Compare September 12, 2022 12:42
@smcgivern
Copy link
Contributor Author

Thanks @pirj, I added some brief development documentation. (Rendered: https://github.com/smcgivern/rubocop-rspec/blob/ad674c8ee2a55a48f933a4605d9145daaa83c7ba/docs/modules/ROOT/pages/development.adoc)

I am not sure what the documentation guidelines are here, so I'm happy to take any and all suggestions there.

@bquorning
Copy link
Collaborator

I have renamed some of the required CI checks, which unfortunately means that you will need to rebase this branch to make CI pass.

@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch from ad674c8 to b98ecc2 Compare September 12, 2022 13:28
@smcgivern
Copy link
Contributor Author

Thanks, done.

@pirj pirj force-pushed the expose-default-rspec-language-context branch from b98ecc2 to b610cea Compare October 12, 2022 20:14
@pirj
Copy link
Member

pirj commented Oct 12, 2022

Rebased and moved the changelist item up to the master section.

@tdeo
Copy link
Contributor

tdeo commented Oct 24, 2022

Fixes #1418, sorry I hadn't look before opening that issue, and thans for adding this support

@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch from b610cea to 8a21113 Compare October 24, 2022 13:06
@tdeo
Copy link
Contributor

tdeo commented Oct 26, 2022

I tried using this branch of rubocop-rspec and it works perfectly for my use-case, following the suggestions in the doc for the setup of metadata and automatic context inclusion is spot on 👌

Otherwise it seems that the CI got stuck here, maybe you can try rebasing on top of master to re-trigger it?

@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch from 8a21113 to 48e0f61 Compare October 26, 2022 10:59
@smcgivern
Copy link
Contributor Author

@tdeo thanks for trying it out!

I rebased because the CHANGELOG had conflicts, but the workflows need a maintainer to approve before they run:

image

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@pirj
Copy link
Member

pirj commented Oct 26, 2022

Green!

@pirj pirj force-pushed the expose-default-rspec-language-context branch from c0f32a8 to 46a3019 Compare November 2, 2022 16:16
@pirj
Copy link
Member

pirj commented Nov 3, 2022

@smcgivern May I ask you to rebase and make sure Changelog is in order?

@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch from 46a3019 to 2291b84 Compare November 3, 2022 16:15
@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch 2 times, most recently from d5c9ae0 to 5335df3 Compare November 3, 2022 16:16
@smcgivern
Copy link
Contributor Author

@pirj sorry for the push spam, I didn't fix the CHANGELOG properly first time around. This is up to date now.

This helper is essential for anyone writing their own RSpec cops based
on `RuboCop::Cop::RSpec::Base`, because it now requires a minimal config
to even run the cop.

The wordy file name is because the RuboCop gem already defines
`rubocop/rspec/shared_contexts`, and we don't want to shadow that. This
means that consumers of this library who want to write a spec for a cop
based on `RuboCop::Cop::RSpec::Base` will need something like this in
`spec/spec_helper.rb`:

require 'rubocop/rspec/shared_contexts/default_rspec_language_config_context'

RSpec.config do |config|
  # ...
  # Add metadata as appropriate to ensure this only happens for RSpec
  # cop specs
  config.include_context 'with default RSpec/Language config'
end
@smcgivern smcgivern force-pushed the expose-default-rspec-language-context branch from 5335df3 to 81c9faa Compare November 3, 2022 16:33
@pirj pirj merged commit 5e7f0fd into rubocop:master Nov 3, 2022
@pirj
Copy link
Member

pirj commented Nov 3, 2022

Thank you, @smcgivern !

@smcgivern smcgivern deleted the expose-default-rspec-language-context branch November 4, 2022 11:22
@smcgivern
Copy link
Contributor Author

Thanks @pirj! I look forward to deleting this file from our repos when this is release 😃

@pirj
Copy link
Member

pirj commented Nov 4, 2022

Released in 2.15.0, @smcgivern

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.

Idea: make spec support available in the gem
5 participants