webmachine / webmachine-ruby

Webmachine, the HTTP toolkit (in Ruby)
http://rdoc.info/github/seancribbs/webmachine-ruby/master/frames
Other
852 stars 54 forks source link

Add the standardrb linter #269

Closed 0x1eef closed 1 year ago

0x1eef commented 1 year ago

standardrb provides a relaxed set of linting rules that provide the benefits of a linter without the headaches that can come from the slightly excessive rubocop defaults.

New rules we'd like to adopt can be enabled in .rubocop.yml, and similarly rules we don't want to adopt can be disabled in .rubocop.yml.

Most rules were auto-correctable, a few were not and uncovered potential bugs. There are some disabled rules in .rubocop.yml - not all of them seem undesirable, but perhaps something to fix in the future.

0x1eef commented 1 year ago

@webmachine/ruby-maintainers What do you think about dropping <= 2.5 from the build matrix ? 2.6, and before that has already reached EOL. It would help bring this PR to a green build. If it is preferred to continue testing against those Ruby versions, I can look into addressing the underlying issue.

bethesque commented 1 year ago

Hi! Yes, I think 2.5 should be dropped. 2.7 is the lowest I'm using (which is embarrassingly old enough imo).

bethesque commented 1 year ago

With regards to the linting changes - I don't feel like I have an ownership level of this codebase that would permit me to approve that, but I think it's fine, and if it was my codebase, I'd say 👍🏽 It's probably @seancribbs call.

seancribbs commented 1 year ago

I didn't see anything objectionable in the style changes. My only question was whether the micro-optimization of single-quoted strings in the case where interpolation is unnecessary still applies. I'm guessing not?

0x1eef commented 1 year ago

My only question was whether the micro-optimization of single-quoted strings in the case where interpolation is unnecessary still applies. I'm guessing not?

I believe single quotes are the rubocop default, but not the standardrb default. I changed the rule to prefer single quotes instead: commit. I can't speak to the performance issue, but I think the difference between the two is so small that the decision can be made based on other criteria.

Yes, I think 2.5 should be dropped. 2.7

Thanks ! Dropped here.

orien commented 1 year ago

We should consider adding a required_ruby_version constraint to the gem spec. As these (or later) changes may introduce incompatibilities with earlier versions of Ruby.

bethesque commented 1 year ago

Good idea.

bethesque commented 1 year ago

This must be our first PR for years 😆 Thanks @orien!