kramdown icon indicating copy to clipboard operation
kramdown copied to clipboard

Bump rubocop to v1.26.x

Open ashmaroli opened this issue 2 years ago • 2 comments

A lot changed since the last time .rubocop.yml in this repository was touched. Since there's no record of what version of Rubocop was used at the time, I decided to go with a newer version.

Primarily, recent versions of RuboCop has dropped static analyses for Ruby 2.3. Therefore, I bumped the required Ruby version to >= 2.5.0. Since I don't know if you'd consider this change fit for a minor semver bump or a major, I limited changes to the lib files in this pull request.

Summary

  • Bump RuboCop to v1.18.x (latest being 1.21).
  • Update RuboCop config for compatibility with used version.
  • Enable all new cops and generate a TODO file.
  • Load rubocop-performance extension as well for obtaining meaningful benefit from this pull request.
  • Fix offenses reported by the Performance/CollectionLiteralInLoop cop.

Additional Notes

Changes are in bite-sized commit for easy reviewing.

Ideally, it would have been better if the immutable literals under focus in this pull request were assigned to constants instead of local variables. But then constants would need to be named aptly, decide whether they need to be public or private...

ashmaroli avatar Sep 20 '21 13:09 ashmaroli

@ashmaroli My thoughts:

  • The code changes regarding the immutable literals: Do the changes make things actually faster?

  • Bumping the needed Ruby version to 2.5 is okay. The documentation should be adjusted to reflect the change.

  • For rubocop it would probably be best to use the latest released version and directly fix the offenses instead of masking them.

gettalong avatar Mar 17 '22 22:03 gettalong

  • The code changes regarding the immutable literals: Do the changes make things actually faster?

These being micro-optimizations wouldn't produce any noticeable improvements in execution times. Off the many offences flagged by RuboCop, I found offenses flagged by Performance/CollectionLiteralInLoop to be the least debatable (least opinionated) to get the ball rolling.

  • For rubocop it would probably be best to use the latest released version and directly fix the offenses instead of masking them.

I've bumped to the current latest RuboCop 1.26. I prefer making atomic changes via multiple PRs to ease reviewing of changes.

ashmaroli avatar Mar 18 '22 06:03 ashmaroli

Hi @ashmaroli! I'm currently looking at kramdown and updating stuff and such, and rubocop was the low hanging fruit for getting started before I saw the pull request. So this is done, including all decisions regarding cops.

Thanks nonetheless for the pull request!

gettalong avatar Mar 14 '23 14:03 gettalong

No worries at my end, @gettalong. Glad to know it's been handled nevertheless.

ashmaroli avatar Mar 14 '23 14:03 ashmaroli