-
-
Notifications
You must be signed in to change notification settings - Fork 729
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 issues with safe auto-correct #11253
Comments
Seems there's still work on this. Reopening |
can you assign this to me please? |
Sure, thank you @Hanaffi 🎉 Welcome to OFN! |
@Hanaffi this may be finished. I was going to double check one more time. If you don't get any changes, it is probably finished. Thanks for contributing though! |
@macanudo527 I was thinking of this from #11820 :
Isn't that related to this? Or are you on working on those? |
@sigmundpetersen sorry about the confusion. Rubocop reports there are 300+ left but when I ran the script it didn't make any corrections. So either the script is broken or, more likely rubocop is reporting what can be unsafely autocorrected. I haven't had time to investigate. In either case, thinking on it more, It's probably best only one developer be assigned to this at a time since we could easily do double work. Can you give me the weekend to wrap it up? @Hanaffi I don't have my laptop handy but there is another issue that lists the important rubocop errors in order of priority. If you'd like to pitch in I would recommend grabbing one of those. I've been working on the rails/ offenses at the moment but there are others you could grab. If you need help, don't be afraid to @ me. |
No worries @macanudo527 , thanks for the explanation. @Hanaffi the issue @macanudo527 mentions is #6055 , have a look and see if there is anything you can start on there. |
@sigmundpetersen |
@dacook should we close this? c.f. discussion above |
Thanks for the prompt. Because it had been a couple of months, I ran the script again. A few more popped up so I've added one last PR to close it. |
Many of the issues listed in .rubocop_todo.yml can be safely auto-corrected, so we will resolve these first. This should cut the list down significantly and clear the scene before getting into more gnarly ones.
We even have a script to automatically auto-correct these, so it should be really easy! But it would be a good idea to chunk up changes and consider each one with a code review (and maybe some will require manual testing).
Process
Execute script/rubocop-autocorrect.sh for a chunk of issues. Eg for 20:
This will generate a commit for each rule, see the first PR for example: #11241
Submit PR and request code review.
To avoid the pain of merge conflicts, wait until it has been merged before starting the next one.
The text was updated successfully, but these errors were encountered: