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

RuboCop Quest Issue #204

Closed
74 of 76 tasks
olleolleolle opened this issue Feb 3, 2020 · 4 comments
Closed
74 of 76 tasks

RuboCop Quest Issue #204

olleolleolle opened this issue Feb 3, 2020 · 4 comments

Comments

@olleolleolle
Copy link
Member

olleolleolle commented Feb 3, 2020

Basic Info

We are linting the whole codebase using RuboCop. ✨

This is a list of all the rules in the .rubocop_todo.yml when we started. The checkmarks mean "this is fixed and merged to master".

If you create a PR to fix one of these, use the --only option in RuboCop, to focus the effort on 1 kind of fix. It's so easy to get overwhelmed when reviewing code like this. (If you fix slightly more than that, no biggie. This is all about keeping the changes reviewable.)

# edit .rubocop.yml, commenting out the first line -
#   this removes the "ignore the TODOs" setting

# Auto-correct with only 1 rule
rubocop -a --only Name/OfTheRuleYouAreFixing --auto-correct

# Re-generate the config file
rubocop --auto-gen-config

# Revert line 3 in .rubocop_todo.yml
# This will avoid PR conflicts

# Inspect to see if it looks OK
git diff

# Re-set the TODO list
git checkout .rubocop.yml

# Add and commit the change
git add .
git commit -m"chore: RuboCop lint Name/OfTheRuleYouAreFixing"
A workflow

Issue description

(Here - we will add each of the RuboCop Cops in order.)

See #200

@iMacTia iMacTia mentioned this issue Feb 3, 2020
4 tasks
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
@iMacTia
Copy link
Member

iMacTia commented Feb 7, 2020

@d-m-u @onk thanks for your patience, I've resolved a bunch of offences automatically and updated the list above 👍
If you want to help, please follow the below:

  1. Leave a comment here before starting, stating the rule(s) you're taking care of
  2. Branch from master into a new branch
  3. Remove the rule(s) you want to fix from the .rubocop_todo.yml file
  4. Run rubocop to see the offenses
  5. Fix the offences
  6. Commit, push and open a PR

Thanks ❤️ !

@d-m-u
Copy link
Contributor

d-m-u commented Feb 7, 2020

I've got the Lint/Loop one.

d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
@d-m-u d-m-u mentioned this issue Feb 7, 2020
@d-m-u
Copy link
Contributor

d-m-u commented Feb 7, 2020

I've also got the Style/GuardClause and the Naming/VariableNumber and the Style/CaseEquality thing and the Naming/ConstantName one, and the Style/NumericPredicate and the Lint/AssignmentInCondition and the MultilineTernaryOperator one. I swear that's it though.

edit fine also the Naming/PredicateName and the Style/IfUnlessModifier and the Layout/LineLength but I expect that one will be needing a few reviews 😆

d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
Gemspec requires ruby min 2.3
from lostisland#223 (comment)

original issue is of course lostisland#204
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
Gemspec requires ruby min 2.3
from #223 (comment)

original issue is of course #204
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
d-m-u added a commit to d-m-u/faraday_middleware that referenced this issue Feb 7, 2020
olleolleolle pushed a commit that referenced this issue Feb 7, 2020
@olleolleolle
Copy link
Member Author

Thank you, everyone! I am declaring a victory: there are now AbcSize + MethodLength left, and those are just set in the TODO file to a current "max" value which we can prune downwards as we go.

This was a significant piece of formatting - thanks so much for stepping up and helping out with this. A special round of applause for @onk and @d-m-u who picked up the bulk of the pieces left after @iMacTia did the initial automated push.

Closing this Issue feels very good. Thanks!

🌻 /Olle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants