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.

Sign up for GitHub

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

Jump to bottom

RuboCop Quest Issue #854

Closed
olleolleolle opened this issue Feb 20, 2019 · 5 comments
Closed

RuboCop Quest Issue #854

olleolleolle opened this issue Feb 20, 2019 · 5 comments
Assignees
@olleolleolle @iMacTia
Labels
help wanted
Projects
v1.0
Milestone
v1.0

Comments

@olleolleolle
Copy link
Member

olleolleolle commented Feb 20, 2019 •
edited

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
This was referenced Feb 20, 2019
Merged
Merged
Merged
Merged
@htwroclau htwroclau mentioned this issue Feb 23, 2019
Merged
This was referenced Feb 24, 2019
Merged
Merged
This was referenced Feb 24, 2019
Closed
Merged
@jherdman jherdman mentioned this issue Feb 25, 2019
Merged
@jherdman
Copy link
Contributor

jherdman commented Feb 25, 2019 •
edited

Removing this comment. My previous assertion was wrong. Future devs should note that .rubocop_todo.yml file when checking for violations.

@jherdman jherdman mentioned this issue Feb 25, 2019
Merged
@technoweenie
Copy link
Member

The current master ref has some rubocop violations not being caught by CI. Those issues are fixed in #870 and #871.

Also, if PRs are named something like [RuboCop] fix Layout/TrailingBlankLines, then they'll be much easier to add to the above checklist.

@olleolleolle olleolleolle mentioned this issue Feb 25, 2019
Merged
@iMacTia iMacTia mentioned this issue Feb 26, 2019
Merged
This was referenced Feb 26, 2019
Merged
Merged
This was referenced Feb 26, 2019
Merged
Merged
@jherdman jherdman mentioned this issue Feb 27, 2019
Merged
This was referenced Feb 27, 2019
Merged
Merged
Merged
@iMacTia iMacTia added this to the v1.0 milestone Feb 28, 2019
@iMacTia iMacTia added this to Backlog in v1.0 via automation Feb 28, 2019
@iMacTia iMacTia moved this from Backlog to In progress in v1.0 Feb 28, 2019
@iMacTia iMacTia mentioned this issue Feb 28, 2019
Merged
@htwroclau htwroclau mentioned this issue Mar 2, 2019
Merged
@olleolleolle olleolleolle mentioned this issue Mar 2, 2019
Merged
This was referenced Mar 2, 2019
Merged
Merged
This was referenced Mar 3, 2019
Closed
Merged
@olleolleolle olleolleolle mentioned this issue Mar 3, 2019
Merged
This was referenced Mar 3, 2019
Merged
Merged
Closed
This was referenced Mar 3, 2019
Merged
Merged
This was referenced Mar 4, 2019
Merged
Merged
This was referenced Mar 4, 2019
Merged
Merged
Merged
This was referenced Mar 4, 2019
Merged
Merged
@iMacTia iMacTia mentioned this issue Mar 5, 2019
Merged
@technoweenie
Copy link
Member

I checked off Style/GlobalVars because the only offender is script/generate_certs. Once the old integration tests have fully been ported, the old scripts for the integration suite can go, and this offense will be magically fixed. Any other global vars added elsewhere should continue to be flagged as rubocop offenses.

@Insti Insti mentioned this issue Mar 9, 2019
Merged
@Insti
Copy link
Contributor

Insti commented Mar 10, 2019

I hadn't seen this post before I fixed the Style/Globalvars 🤷‍♂️

@iMacTia iMacTia mentioned this issue Mar 13, 2019
Closed
@technoweenie technoweenie mentioned this issue Mar 31, 2019
Closed
7 tasks
@olleolleolle olleolleolle mentioned this issue Apr 8, 2019
Merged
v1.0 automation moved this from In progress to Done Apr 8, 2019
@olleolleolle
Copy link
Member Author

Thanks, everyone! 🚀

@technoweenie technoweenie mentioned this issue Jun 25, 2019
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@olleolleolle olleolleolle

@iMacTia iMacTia

Labels
help wanted
Projects
No open projects
v1.0
Done
Milestone
v1.0
Development

No branches or pull requests
5 participants
@technoweenie @olleolleolle @jherdman @iMacTia @Insti