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

Config formatter coverage #2031

Merged
merged 3 commits into from
Feb 10, 2025
Merged

Config formatter coverage #2031

merged 3 commits into from
Feb 10, 2025

Conversation

bquorning
Copy link
Collaborator

@bquorning bquorning commented Feb 9, 2025

SUBDEPARTMENTS was last used in ConfigFormatter before we released v3.0.0, before RSpec/Capybara was extracted. It’s being removed in this PR.

Also, initially wanting to improve spec branch coverage for ConfigFormatter, I found that AMENDMENTS was defined as a String, not an Array. And splatting the array into #gsub doesn't work if there is more than one element - we need to use Regexp.union instead. Actually I ended up removing AMENDMENDS altogether.

To avoid conflicts with other PRs improving code coverage, the .simplecov file is not updated here.


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

@bquorning bquorning requested a review from a team as a code owner February 9, 2025 12:53
@bquorning
Copy link
Collaborator Author

cc @corsonknowles

@bquorning bquorning added this to the Code coverage milestone Feb 9, 2025
@corsonknowles
Copy link
Contributor

Thanks! I was about to raise an issue on this because of the String/Array mismatch. Glad you knew how to fix it!

@@ -8,6 +8,12 @@
'AllCops' => {
'Setting' => 'forty two'
},
'Metrics/BlockLength' => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice the spec explains the behavior now!

@bquorning bquorning force-pushed the config-formatter-coverage branch from 6764c53 to 73bc726 Compare February 9, 2025 19:53
Its last use was before v3.0.0, before `RSpec/Capybara` was extracted.
AMENDMENTS was defined as a String, not an Array. And splatting the
array into `#gsub` doesn't work if there is more than one element - we
would need to use `Regexp.union` instead...

but actually I ended up removing AMENDMENTS altogether. All we want to
do is to add an extra newline before each cop entry. Only new cop
entries starts at the beginning of line, and contains a slash character
after the first word. This means we don't need AMENDMENTS at all, and we
no longer need the capture in EXTENSION_ROOT_DEPARTMENT.
@bquorning bquorning force-pushed the config-formatter-coverage branch from 79a329f to e79ec49 Compare February 9, 2025 20:10
@bquorning bquorning requested a review from pirj February 9, 2025 21:18
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!

It was such a pain to make this work. I'm happy we don't need this code anymore.

image

@bquorning bquorning merged commit 285d92b into master Feb 10, 2025
27 checks passed
@bquorning bquorning deleted the config-formatter-coverage branch February 10, 2025 09:44
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.

3 participants