Drop support for older rubies, rails and redis #566

fatkodima · 2024-11-15T01:10:01Z

Also a candidate for a new major release, imo.

This PR drops support for:

ruby < 3.1 (https://devcenter.heroku.com/articles/ruby-support-reference#supported-runtimes, https://endoflife.date/ruby)
rails < 7
redis < 6 (https://redis.io/docs/latest/operate/rs/installing-upgrading/product-lifecycle/).

It also updates CI to check against newer railses.

There is a mention about jruby in the readme, but no tests on CI. I would suggest just dropping it, as I am not sure many people (anyone?) uses it in production to worth the effort.

fatkodima · 2024-11-15T12:31:48Z

I also noticed, that there is a rubocop and standardrb. I would suggest removing the latter and properly use rubocop and fix all the current offenses.

Also, there is a

coverband/Rakefile

Line 11 in 204d2d8

task "test:all": %i[rubocop test forked_tests benchmarks:memory benchmarks]

task, which runs forked_tests. These are not ran on CI currently and are broken. That should be also fixed.

I can help, if ok.

danmayer · 2024-11-18T00:00:47Z

Gemfile.rails7.2

@@ -4,7 +4,7 @@ source 'https://rubygems.org'

 # Specify your gem's dependencies in coverband.gemspec
 gemspec
-gem 'rails', '~>7'


we might want a rails7 rails 7.1 and rails 7.2

yes, we have gemfiles for 7.0, 7.1 and 7.2, thats github incorrectly displays this change as a renamed file 🤷

danmayer · 2024-11-18T00:03:11Z

lib/coverband/adapters/hash_redis_store.rb

@@ -25,14 +25,14 @@ def initialize(redis, opts = {})
        @save_report_batch_size = opts[:save_report_batch_size] || 100
        @format_version = REDIS_STORAGE_FORMAT_VERSION
        @redis = redis
-        raise "HashRedisStore requires redis >= 2.6.0" unless supported?
+        raise "HashRedisStore requires redis >= 6.0.0" unless supported?


Do we have a reason to require redis 6, we were checking this based on features that redis supports but we aren't using any of the more modern and newer redis features.

There is no reason, only as encouraging people upgrading their redis. I also doubt that people are on the latest version of coverband, but on the redis which is EOL many years ago. But happy to revert.

danmayer · 2024-11-18T00:03:55Z

lib/coverband/collectors/route_tracker.rb

@@ -13,7 +13,7 @@ class RouteTracker < AbstractTracker
      TITLE = "Routes"

      def initialize(options = {})
-        if Rails&.respond_to?(:version) && Gem::Version.new(Rails.version) >= Gem::Version.new("6.0.0") && Gem::Version.new(Rails.version) < Gem::Version.new("7.1.0")
+        if Rails&.respond_to?(:version) && Gem::Version.new(Rails.version) < Gem::Version.new("7.1.0")
          require_relative "../utils/rails6_ext"


in this case we can delete this check and delete the rails6_ext file

We still support rails 7.0, but the feature was added in 7.1 (rails/rails#43755)

danmayer · 2024-11-18T00:05:32Z

Thanks... I guess we could start putting together a major release.

I am good with updating to all the latest supported versions
The JRuby tests were flaky and causing issues when I moved to github actions, but the support was added due to some requests, so I don't want to drop it. We can either fix the tests or add a comment that it is no longer tested
I will try to fix the forked tests on CI they worked with a small tweak on my machine.

on Rubocop and Standard, I prefer standardrb for all my projects, I have rubocop as some editor tool chains seem to struggle with using standard as a auto formatter. Rubocop is supposed to use standard via the config file but appears not to do so...

If we are going to remove one, I would remove all the references to rubocop
CI enforces standardrb passing
standard is currently passing

danmayer@dellstation:~/projects/coverband$ bundle exec standardrb
Inspecting 153 files
.........................................................................................................................................................

danmayer · 2024-11-18T00:45:36Z

I fixed the forked tests, I also fixed up the benchmark tasks.... those are all now running on CI.

See the few comments I left after that I am happy to merge this.

fatkodima

Updated PR with removing old dummy apps from test/ directory.

fatkodima · 2024-11-18T09:57:10Z

Gemfile.rails7.2

@@ -4,7 +4,7 @@ source 'https://rubygems.org'

 # Specify your gem's dependencies in coverband.gemspec
 gemspec
-gem 'rails', '~>7'


yes, we have gemfiles for 7.0, 7.1 and 7.2, thats github incorrectly displays this change as a renamed file 🤷

fatkodima · 2024-11-18T09:58:40Z

lib/coverband/adapters/hash_redis_store.rb

@@ -25,14 +25,14 @@ def initialize(redis, opts = {})
        @save_report_batch_size = opts[:save_report_batch_size] || 100
        @format_version = REDIS_STORAGE_FORMAT_VERSION
        @redis = redis
-        raise "HashRedisStore requires redis >= 2.6.0" unless supported?
+        raise "HashRedisStore requires redis >= 6.0.0" unless supported?


There is no reason, only as encouraging people upgrading their redis. I also doubt that people are on the latest version of coverband, but on the redis which is EOL many years ago. But happy to revert.

fatkodima · 2024-11-18T10:21:35Z

lib/coverband/collectors/route_tracker.rb

@@ -13,7 +13,7 @@ class RouteTracker < AbstractTracker
      TITLE = "Routes"

      def initialize(options = {})
-        if Rails&.respond_to?(:version) && Gem::Version.new(Rails.version) >= Gem::Version.new("6.0.0") && Gem::Version.new(Rails.version) < Gem::Version.new("7.1.0")
+        if Rails&.respond_to?(:version) && Gem::Version.new(Rails.version) < Gem::Version.new("7.1.0")
          require_relative "../utils/rails6_ext"


We still support rails 7.0, but the feature was added in 7.1 (rails/rails#43755)

fatkodima · 2024-11-18T10:42:32Z

I honestly prefer rubocop over standardrb, because I like full control and also when trying to ignore something with standardrb it starts looking like a hack for me. So I suggest having only one of them. If you want to remove one of them, please do, or let me know and I can do this.

danmayer · 2024-11-18T18:59:12Z

There is no reason, only as encouraging people upgrading their redis. I also doubt that people are on the latest version of coverband, but on the redis which is EOL many years ago. But happy to revert.

yeah let's leave it with the older redis support... Many users of Coverband are trying to use it because they are maintaining old legacy apps, so we often have users that are way behind the curve in terms of updates. You can see various issues people open wanting support for older Ruby and Rails versions... I want to be able to update but I do try to keep a pretty wide support window for that reason.

I honestly prefer rubocop over standardrb...

yeah I understand on rubocop vs standardrb, everyone has some favorites there, I am more on the standardrb side of things as I don't want to tweak things to much... but I can try to totally remove rubocop as it seems like running Rubocop with the standardrb mode doesn't really work correctly. I will take a look at cleaning that up tonight.

fatkodima · 2024-11-18T19:33:08Z

Reverted redis changes.

danmayer · 2024-11-20T03:22:15Z

Thanks for all of this clean up, merged this main branch will be part of a major version release.

danmayer reviewed Nov 18, 2024

View reviewed changes

fatkodima force-pushed the drop-old-dependencies branch from dd01a33 to 5ef4d19 Compare November 18, 2024 10:33

fatkodima commented Nov 18, 2024

View reviewed changes

Drop support for older rubies, rails and redis

1aa3cd6

fatkodima force-pushed the drop-old-dependencies branch from 5ef4d19 to 1aa3cd6 Compare November 18, 2024 19:32

danmayer approved these changes Nov 20, 2024

View reviewed changes

danmayer merged commit c562891 into danmayer:main Nov 20, 2024
47 of 48 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop support for older rubies, rails and redis #566

Drop support for older rubies, rails and redis #566

fatkodima commented Nov 15, 2024

fatkodima commented Nov 15, 2024 •

edited

Loading

danmayer Nov 18, 2024

fatkodima Nov 18, 2024

danmayer Nov 18, 2024

fatkodima Nov 18, 2024

danmayer Nov 18, 2024

fatkodima Nov 18, 2024

danmayer commented Nov 18, 2024

danmayer commented Nov 18, 2024

fatkodima left a comment

fatkodima Nov 18, 2024

fatkodima Nov 18, 2024

fatkodima Nov 18, 2024

fatkodima commented Nov 18, 2024

danmayer commented Nov 18, 2024 •

edited

Loading

fatkodima commented Nov 18, 2024

danmayer commented Nov 20, 2024

Drop support for older rubies, rails and redis #566

Drop support for older rubies, rails and redis #566

Conversation

fatkodima commented Nov 15, 2024

fatkodima commented Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danmayer commented Nov 18, 2024

danmayer commented Nov 18, 2024

fatkodima left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fatkodima commented Nov 18, 2024

danmayer commented Nov 18, 2024 • edited Loading

fatkodima commented Nov 18, 2024

danmayer commented Nov 20, 2024

fatkodima commented Nov 15, 2024 •

edited

Loading

danmayer commented Nov 18, 2024 •

edited

Loading