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 rubocop_todo link injection when YAML doc start sigil exists #9406
Conversation
a05db32
to
28c37f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks for the fix!
f.write "inherit_from:#{file_string}\n" | ||
f.write "\n#{rubocop_yml_contents}" if /\S/.match?(rubocop_yml_contents) | ||
end | ||
lines = /\S/.match?(rubocop_yml_contents) ? rubocop_yml_contents.split("\n", -1) : [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity and simplicity:
lines = /\S/.match?(rubocop_yml_contents) ? rubocop_yml_contents.split("\n", -1) : [] | |
lines = rubocop_yml_contents.blank? ? [] : rubocop_yml_contents.split("\n", -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion @dvandersluis. Unfortunately I see test failures locally with this suggestion (via rspec ./spec/rubocop/cli/cli_auto_gen_config_spec.rb
). I think it's due to the nonequivalence when rubocop_yml_contents
is nil
. (Specifically, rubocop's blank?
monkey-patch differs from rails by being undefined on nil
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah looks like that's because rubocop_yml_contents
can be nil
if the file doesn't exist because it's not otherwise initialized in add_inheritance_from_auto_generated_file
🤔
Can anyone help me understand how to make |
I've seen this intermittently on other PRs but it often is not persistent. It won't block merging this PR. |
Thank you! |
The
rubocop --auto-gen-config
script will insertinherit_from: .rubocop_todo.yml
at the top of the rubocop config file if the line doesn't already exist. However, if the config file has a YAML document start sigil (---
), this will elide the remainder of the config file when running rubocop. This PR resolves the issue by injecting theinherit_from
entry after the first such sigil, if one exists.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.