RuboCop corrections
What does this MR do and why?
Follow up from !303 (merged)
Closes #340 (closed)
Refs #343 (closed) - I think this might also be solved by this.
In !303 (merged), we introduced a big rubocop_todo.yml file
In this MR, we clean up a majority of it whilst still remaining rather confident the code all works.
- All safe cops are confirmed working
- Many unsafe cops are handled
- Some required a little bit of manual intervention, I tried to include a
!
in these commits, but one or two commits might've slipped through unmarked. - Please pay extra attention to FrozenStringLiterals, as those could cause complications
- Some commits get outdated later in the chain, as I have 2 wrap up commits
- Some cops had to be disabled, I provided a rationale when needed. They were either too much effort to fix, not feasible, not a decision I could make, or would fail too many specs.
- Some required a little bit of manual intervention, I tried to include a
- Hopefully my self-review is helpful to pointing out some risky areas
Note
This MR changes a lot of stuff, please consider not merging other significant changes (to avoid conflicts) - I'm happy to provide post-merge support should something pop up, I'd rather just not deal with complicated merge conflicts.
Test Plan
It's important to note that I did some quick checks on if this all still works, but I would encourage a real end to end test plan before a release. The specs are green, rubocop is happy, but it wouldn't surprise me if edge cases stopped working I did not consider.
My biggest fear is around frozen string literals.
Follow first
This MR is an extract from this one - it doesn't strictly need to be merged first but may offer additional context if handled first. The conflict itself should be minor and easy to handle.
Follow ups
- !312 (merged) (Draft: introduce .git-blame-ignore-revs)
- !313 (merged) (Draft: Enforce MFA for Gem upload)