knowndecimal / fulfil

Interact with the Fulfil.io API using Ruby
MIT License
9 stars 2 forks source link

Add rubocop #35

Closed stefanvermaas closed 2 years ago

stefanvermaas commented 2 years ago

Thought it would be helpful if we'd add rubocop to the project to improve the code styles.

The current configuration uses the default configuration of Rubocop. It also includes two extra extensions:

I've ran rubocop --autocorrect-all already. It adds some noise, but I think it's doable. I've also generated a .rubocop_todo.yml to address non-autocorrectable notifications in a follow-up PR.

stefanvermaas commented 2 years ago

Hmm, rubocop doesn't support Ruby 2.4 and 2.5 anymore. Which I guess is fair as these are EOL Ruby versions.

I guess we've got two options here:

  1. We install rubocop (and other gems) in the CI actions itself and don't depend on the gemspec as that will install rubocop (and others) also when we want to run the tests.
  2. We drop support for EOL Ruby versions 2.4 and 2.5.

We currently don't use newer Ruby language features (newer as in 2.4+), so I guess it's more non-invasive to install Rubocop in the CI actions themselves. However, Ruby 2.4 and 2.5 are EOL, so we might as well not support them anymore.

Thoughts?

stefanvermaas commented 2 years ago

LOL.

I just argued we're not using any Ruby language features that were not implemented in < 2.6. However, as you can see in this failing CI action we could be using new language features.

Apparently, a non-explicit begin/rescue/ensure block isn't supported in Ruby < 2.6. Maybe we have a compelling reason to drop support for lower Ruby versions.

We might want to consider dropping support after all.

cdmwebs commented 2 years ago

We drop support for EOL Ruby versions 2.4 and 2.5.

Approved.