-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
# Address false positive when reversing remove_foreign_key #205
Conversation
dff35b8
to
52b1e92
Compare
@@ -10,6 +10,7 @@ | |||
|
|||
* [#180](https://github.com/rubocop-hq/rubocop-rails/issues/180): Fix a false positive for `HttpPositionalArguments` when using `get` method with `:to` option. ([@koic][]) | |||
* [#193](https://github.com/rubocop-hq/rubocop-rails/issues/193): Make `Rails/EnvironmentComparison` aware of `Rails.env` is used in RHS or when `!=` is used for comparison. ([@koic][]) | |||
* [#205](https://github.com/rubocop-hq/rubocop-rails/pull/205): Make `Rails/ReversibleMigration` aware of `:to_table` option of `remove_foreign_key`. ([@joshpencheon][]) |
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.
Can you fix the following failure test?
Failures:
1) RuboCop Rails Project changelog has link definitions for all implicit links
Failure/Error:
expect(changelog.include?("[#{name}]: http"))
.to be(true), "CHANGELOG.md is missing a link for #{name}. " \
'Please add this link to the bottom of the file.'
CHANGELOG.md is missing a link for @joshpencheon. Please add this link to the bottom of the file.
# ./spec/project_spec.rb:27:in `block (4 levels) in <top (required)>'
# ./spec/project_spec.rb:26:in `each'
# ./spec/project_spec.rb:26:in `block (3 levels) in <top (required)>'
Finished in 1.85 seconds (files took 1.32 seconds to load)
2239 examples, 1 failure
https://circleci.com/gh/rubocop-hq/rubocop-rails/2700
You can see also CONTRIBUTING.md:
If this is your first contribution to RuboCop project, add a link definition for the implicit link to the bottom of the changelog as
[@username]: https://github.com/username
.
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.
Done thanks - sorry, the local suite didn't have that failure.
@@ -169,6 +169,11 @@ def change | |||
remove_foreign_key :accounts, :branches | |||
RUBY | |||
|
|||
it_behaves_like 'accepts', | |||
'remove_foreign_key(with to_table option)', <<-RUBY |
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.
Can you update with squiggly heredoc?
'remove_foreign_key(with to_table option)', <<-RUBY | |
'remove_foreign_key(with to_table option)', <<~RUBY |
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.
I found with squiggly heredoc another style rule complained about indentation (see other >80 char workarounds earlier in the file), but have shortened spec label and it fits now.
52b1e92
to
3247a74
Compare
@@ -169,6 +169,10 @@ def change | |||
remove_foreign_key :accounts, :branches | |||
RUBY | |||
|
|||
it_behaves_like 'accepts', 'remove_foreign_key(with table option)', <<~RUBY |
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.
Would you update with the description using :to_table
? This suggestion is less than 80 characters :-)
it_behaves_like 'accepts', 'remove_foreign_key(with table option)', <<~RUBY | |
it_behaves_like 'accepts', 'remove_foreign_key(with `:to_table`)', <<~RUBY |
The other table can be specified either as the second argument, or in the :to_table key of a hash as the second argument. The latter was not supported.
3247a74
to
f851ed9
Compare
Thanks! |
The other table can be specified either as the second argument, or in the :to_table key of a hash as the second argument. The latter was not supported.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).and description in grammatically correct, complete sentences.
bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.