Rebase rubocop branch #1156
Rebase rubocop branch #1156
Conversation
- Add `.rubocop.yml` + `.rubocop_todo.yml` following instructions in https://rubocop.readthedocs.io/en/latest/configuration/#automatically-generated-configuration - Add rake task for rubocop
- Rubocop complained about invalid ruby; since the file is technically erb and _not_ ruby, the extension should reflect that anyway.
- Remove empty lines per `Layout/EmptyLines` Cop. - Remove empty lines per `Layout/EmptyLines` Cop from test-dummy rails app. - Remove `Layout/EmptyLines` Cop from todo list.
- Fix quotes in string literals per `Style/StringLiterals` Cop. - Fix quotes in string literals per rubocop for test-dummy rails app. - Remove `Style/StringLiterals` Cop from todo list.
- This Cop is for future Ruby. Skip it for now; it works with the `--auto-correct` option, making future application trivial.
- This Cop forces either nested or compact module declarations. While nested may be considered best practice, changing these up may lead to all kinds of merge conflicts on open PRs.
- Move `Style/HashSyntax` from todos to main config, and
set enforced style to `ruby19`--i.e. `{foo: :bar}`.
- Update hash syntax from hash-rockets to ruby 1.9 style.
- Update hash syntax to ruby 1.9 style in test-dummy rails app.
- Move `Style/SymbolArray` from todos to main config:
* Set enforced style to 'percent' -- i.e. `%i[]`.
* Don't enforce on arrays with < 3 elements.
- Use percent array literals for large arrays of symbols.
- Use percent array literals for large arrays of symbols.
When `rubocop` was added, the `parser` gem was added as a dependency. Unfortunately, the version of parser specified in the `Gemfile.lock` was unpublished; must update to the newly released parser dependency.
- Custom configuration for cop: * Set max to 25 lines (default) * Exclude Rails Engine file * Exclude custom DSL files from test-dummy Rails app * Exclude `describe` blocks in tests * Exclude routing blocks - Exclude `app/models/devise_token_auth/concerns/user.rb` in `.rubocop_todo.yml`. NOTE: This file could use a bunch of refactoring. - Resolve violation in `test/controllers/overrides/passwords_controller_test.rb` by removing unused or single-use instance variables
- Also partially fixes Style/BracesAroundHashParameters
- Enforce for array literals of 3 or more elements
- Ignore violations for minitest assertion methods.
- Custom configuration for cop:
* Set max to 80 columns
* Exclude all test files
* Exclude `Guardfile` & `gemspec`
* Ignore comments
- Exclude remaining files with violations via `.rubocop_todo.yml`.
NOTE: Some files need refactoring, others can be resolved by
extending max LineLength from 80 to 100 columns.
- Resolve some violations where obvious.
…tive_to_receiver`
rubocop --auto-gen-config --exclude-limit 100
|
@MaicolBen |
.rubocop_todo.yml
Outdated
| - 'lib/devise_token_auth/engine.rb' | ||
| - 'lib/devise_token_auth/rails/routes.rb' | ||
| - 'lib/generators/devise_token_auth/install_generator.rb' | ||
| Max: 189 |
There was a problem hiding this comment.
The --auto-gen-config option sets this... It picks whatever line-length necessary so that everything is ignored. Hence, I set it explicitly back to the default of Max: 80 in the .rubocop.yml file. I then manually exclude each file with a violation, instead of extending the max length.
There was a problem hiding this comment.
I just realized that this is a todo file, I commented this also because #1125 sets 80. Will you remove this file in the future?
| # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. | ||
| # URISchemes: http, https | ||
| Metrics/LineLength: | ||
| Max: 189 |
|
@MaicolBen, as I understand 189 is length of the most longest line in whole project. Check it with command: rubocop --only Metrics/LineLengthI don't know why exclude list of Metrics/LineLength cop is not shown. The lines with length more than 80 are persisted in the project and Metrics/LineLength cop was not fixed totally. @Evan-M What do you think about current PR and why 189? |
Evan-M
force-pushed
the
add_rubocop
branch
3 times, most recently
from
b41294b to
c19432b
Compare
|
I see the purpose of the todo file, the idea is once is everything is fixed, that file is removed, when you rebase again :/, I will merge |
|
Whoops, I thought you were talking about merging #1125. UPDATE: I've actually cherry-picked a2a67f0 and 4be88a1 into #1125, and added via So this PR probably doesn't need to be merged. Note: that rebasing the |
|
@Evan-M So can we close this one? |
Evan-M
force-pushed
the
add_rubocop
branch
2 times, most recently
from
d20cb34 to
994eb85
Compare
|
Closing. See #1156 (comment). |
Reference #1125