DEV: Fix linting (#353)

Moves linting to a separate CI step and removes rubocop from the Rakefile (at least until we drop Ruby 2.6)

Author: CvX

.github/workflows/ci.yml

@@ -7,7 +7,34 @@ on:
   pull_request:
 
 jobs:
-  build:
+  lint:
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: "3.3"
+          bundler-cache: true
+
+      - name: Set up Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 20
+          cache: npm
+
+      - name: Setup npm
+        run: npm install
+
+      - name: Rubocop
+        run: bundle exec rubocop
+
+      - name: ESLint
+        run: npx eslint .
+
+  test:
     runs-on: ubuntu-latest
     name: Ruby ${{ matrix.ruby }} (redis ${{ matrix.redis }})
     timeout-minutes: 10
@@ -20,7 +47,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: [2.6, 2.7, '3.0', 3.1]
+        ruby: [2.6, 2.7, "3.0", 3.1]
         redis: [5, 6]
 
     services:
@@ -50,27 +77,14 @@ jobs:
           ruby-version: ${{ matrix.ruby }}
           bundler-cache: true
 
-      - name: Set up Node.js
-        uses: actions/setup-node@v3
-        with:
-          node-version: 18
-          cache: npm
-
-      - name: Setup npm
-        run: npm install
-
       - name: Tests
         env:
           TESTOPTS: --verbose
         run: bundle exec rake
-        timeout-minutes: 3
-
-      - name: Linting
-        run: npx eslint .
 
   publish:
     if: github.event_name == 'push' && github.ref == 'refs/heads/main'
-    needs: build
+    needs: [lint, test]
     runs-on: ubuntu-latest
 
     steps:

.rubocop.yml

@@ -1,11 +1,15 @@
 inherit_gem:
-  rubocop-discourse: .rubocop.yml
+  rubocop-discourse: stree-compat.yml
 inherit_mode:
   merge:
     - Exclude
+
 AllCops:
   Exclude:
-    - 'examples/**/*'
+    - "examples/**/*"
+
+Discourse/Plugins:
+  Enabled: false
 
 RSpec:
   Enabled: false

Rakefile

@@ -4,12 +4,10 @@ require 'rake/testtask'
 require 'bundler'
 require 'bundler/gem_tasks'
 require 'bundler/setup'
-require 'rubocop/rake_task'
 require 'yard'
 
 Bundler.require(:default, :test)
 
-RuboCop::RakeTask.new
 YARD::Rake::YardocTask.new
 
 BACKENDS = Dir["lib/message_bus/backends/*.rb"].map { |file| file.match(%r{backends/(?<backend>.*).rb})[:backend] } - ["base"]
@@ -99,5 +97,5 @@ task :performance do
   end
 end
 
-desc "Run all tests, link checks and confirms documentation compiles without error"
-task default: [:spec, :rubocop, :test_doc]
+desc "Run all tests and confirm the documentation compiles without error"
+task default: [:spec, :test_doc]

lib/message_bus.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "monitor"
-require "set"
 
 require_relative "message_bus/version"
 require_relative "message_bus/message"
@@ -772,7 +771,7 @@ def global_subscribe_thread
         globals, locals, local_globals, global_globals = nil
 
         @mutex.synchronize do
-          return if @destroyed
+          return if @destroyed # rubocop:disable Lint/NonLocalExitFromIterator
           next unless @subscriptions
 
           globals = @subscriptions[nil]

lib/message_bus/backends/base.rb

@@ -48,7 +48,6 @@ module Backends
     #
     # @abstract
     class Base
-      # rubocop:disable Lint/UnusedMethodArgument
 
       # Raised to indicate that the concrete backend implementation does not implement part of the API
       ConcreteClassMustImplementError = Class.new(StandardError)

lib/message_bus/backends/postgres.rb

@@ -406,7 +406,7 @@ def global_subscribe(last_id = nil)
             on.message do |_c, m|
               if m == UNSUB_MESSAGE
                 @subscribed = false
-                return
+                return # rubocop:disable Lint/NonLocalExitFromIterator
               end
               m = MessageBus::Message.decode m
 

lib/message_bus/backends/redis.rb

@@ -300,7 +300,7 @@ def global_subscribe(last_id = nil, &blk)
               if m == UNSUB_MESSAGE
                 @subscribed = false
                 global_redis.unsubscribe
-                return
+                return # rubocop:disable Lint/NonLocalExitFromIterator
               end
               m = MessageBus::Message.decode m
 

message_bus.gemspec

@@ -35,6 +35,8 @@ Gem::Specification.new do |gem|
   gem.add_development_dependency 'byebug'
   gem.add_development_dependency 'oj'
   gem.add_development_dependency 'yard'
-  gem.add_development_dependency 'rubocop-discourse'
-  gem.add_development_dependency 'rubocop-rspec'
+
+  if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7.0')
+    gem.add_development_dependency 'rubocop-discourse', '3.8.1'
+  end
 end

spec/lib/message_bus/multi_process_spec.rb

@@ -22,7 +22,7 @@ def work_it
       if msg.data == "done"
         bus.global_unsubscribe
       else
-        bus.publish("/response", "#{msg.data}-#{Process.pid.to_s}")
+        bus.publish("/response", "#{msg.data}-#{Process.pid}")
       end
     end
   ensure

spec/spec_helper.rb

@@ -19,7 +19,7 @@
 puts "Running with backend: #{CURRENT_BACKEND}"
 
 def test_only(*backends)
-  skip "Test doesn't apply to #{CURRENT_BACKEND}" unless backends.include?(CURRENT_BACKEND)
+  skip "Test doesn't apply to #{CURRENT_BACKEND}" if backends.exclude?(CURRENT_BACKEND)
 end
 
 def test_never(*backends)