Merge pull request #25 from Shopify/ec-tweaks

Edouard-chin · web-flow · commit f714e62ebd08 · 2025-11-26T15:43:48.000+01:00
Various tweaks
diff --git a/.github/actions/cibuildgem/package.json b/.github/actions/cibuildgem/package.json
@@ -3,12 +3,12 @@
   "version": "1.0.0",
   "description": "A companion GitHub action for the cibuildgem ruby tool.",
   "main": "index.js",
-  "repository": "https://github.com/rails/cool-stuff-fun-time",
+  "repository": "https://github.com/shopify/cibuildgem",
   "scripts": {
     "test": "NODE_OPTIONS=--experimental-vm-modules NODE_NO_WARNINGS=1 jest",
     "package": "ncc build src/index.js -o dist"
   },
-  "author": "Edouard Chin",
+  "author": "Shopify",
   "license": "MIT",
   "devDependencies": {
     "@vercel/ncc": "^0.38.4",
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -2,13 +2,28 @@ name: "Run the test suite"
 on: push
 
 jobs:
+  lint:
+    timeout-minutes: 5
+    name: "Ruby linter"
+    runs-on: ubuntu-latest
+    steps:
+      - name: "Checkout code"
+        uses: "actions/checkout@v5"
+      - name: "Setup ruby"
+        uses: "ruby/setup-ruby@v1"
+        with:
+          bundler-cache: true
+      - name: "Run rubocop"
+        run: "bundle exec rubocop"
   test:
     timeout-minutes: 10
     name: "Testing the gem"
     runs-on: ubuntu-latest
     steps:
       - name: "Checkout code"
         uses: "actions/checkout@v5"
+      - name: "Remove the Gemfile.lock"
+        run: "rm -rf Gemfile.lock"
       - name: "Setup ruby"
         uses: "ruby/setup-ruby@v1"
         with:
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -0,0 +1,8 @@
+inherit_gem:
+  rubocop-shopify: rubocop.yml
+
+AllCops:
+  NewCops: disable
+  SuggestExtensions: false
+  Exclude:
+    - test/fixtures/date/**/*
diff --git a/Gemfile b/Gemfile
@@ -7,3 +7,4 @@ gemspec
 
 gem "rake", "~> 13.0"
 gem "minitest", "~> 5.16"
+gem "rubocop-shopify"
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -9,12 +9,43 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
+    ast (2.4.3)
+    json (2.16.0)
+    language_server-protocol (3.17.0.5)
+    lint_roller (1.1.0)
     minitest (5.25.5)
+    parallel (1.27.0)
+    parser (3.3.10.0)
+      ast (~> 2.4.1)
+      racc
     prism (1.6.0)
+    racc (1.8.1)
+    rainbow (3.1.1)
     rake (13.3.0)
     rake-compiler (1.3.0)
       rake
+    regexp_parser (2.11.3)
+    rubocop (1.81.7)
+      json (~> 2.3)
+      language_server-protocol (~> 3.17.0.2)
+      lint_roller (~> 1.1.0)
+      parallel (~> 1.10)
+      parser (>= 3.3.0.2)
+      rainbow (>= 2.2.2, < 4.0)
+      regexp_parser (>= 2.9.3, < 3.0)
+      rubocop-ast (>= 1.47.1, < 2.0)
+      ruby-progressbar (~> 1.7)
+      unicode-display_width (>= 2.4.0, < 4.0)
+    rubocop-ast (1.48.0)
+      parser (>= 3.3.7.2)
+      prism (~> 1.4)
+    rubocop-shopify (2.18.0)
+      rubocop (~> 1.62)
+    ruby-progressbar (1.13.0)
     thor (1.4.0)
+    unicode-display_width (3.2.0)
+      unicode-emoji (~> 4.1)
+    unicode-emoji (4.1.0)
 
 PLATFORMS
   arm64-darwin-23
@@ -24,6 +55,7 @@ DEPENDENCIES
   cibuildgem!
   minitest (~> 5.16)
   rake (~> 13.0)
+  rubocop-shopify
 
 BUNDLED WITH
    2.7.2
diff --git a/LICENSE.txt b/LICENSE.txt
@@ -1,6 +1,6 @@
 The MIT License (MIT)
 
-Copyright (c) 2025 Edouard CHIN
+Copyright (c) 2025 Shopify
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal
diff --git a/Rakefile b/Rakefile
@@ -13,4 +13,4 @@ rescue LoadError
   # it the GitHub action clones the repo and runs `rake install`, without running a `bundle install` first.
 end
 
-task default: :test
+task(default: :test)
diff --git a/cibuildgem.gemspec b/cibuildgem.gemspec
@@ -30,7 +30,7 @@ Gem::Specification.new do |spec|
   spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) }
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "rake-compiler"
-  spec.add_dependency "thor"
-  spec.add_dependency "prism"
+  spec.add_dependency("prism")
+  spec.add_dependency("rake-compiler")
+  spec.add_dependency("thor")
 end
diff --git a/lib/cibuildgem/cli.rb b/lib/cibuildgem/cli.rb
@@ -9,8 +9,10 @@ class CLI < Thor
 
     source_root(File.expand_path("templates", __dir__))
 
-    def self.exit_on_failure?
-      true
+    class << self
+      def exit_on_failure?
+        true
+      end
     end
 
     desc "compile", "Compile a gem's native extension."
@@ -92,13 +94,12 @@ def clobber
     method_option "working-directory", type: "string", required: false, desc: "If your gem lives outside of the repository root, specify where."
     method_option "test-command", type: "string", required: false, desc: "The test command to run. Defaults to running `bundle exec rake test` and `bundle exec rake spec`."
     def ci_template
-      # os = ["macos-latest", "macos-15-intel", "ubuntu-latest", "windows-latest"]
-      os = ["macos-latest", "ubuntu-22.04"] # Just this for now because the CI takes too long otherwise.
-
       ruby_requirements = compilation_task.gemspec.required_ruby_version
-      latest_supported_ruby_version = RubySeries.latest_version_for_requirements(ruby_requirements)
-      runtime_version_for_compilation = RubySeries.runtime_version_for_compilation(ruby_requirements)
-      ruby_versions_for_testing = RubySeries.versions_to_test_against(ruby_requirements)
+      # os = ["macos-latest", "macos-15-intel", "ubuntu-latest", "windows-latest"]
+      @os = ["macos-latest", "ubuntu-22.04"] # Just this for now because the CI takes too long otherwise.
+      @latest_supported_ruby_version = RubySeries.latest_version_for_requirements(ruby_requirements)
+      @runtime_version_for_compilation = RubySeries.runtime_version_for_compilation(ruby_requirements)
+      @ruby_versions_for_testing = RubySeries.versions_to_test_against(ruby_requirements)
 
       directory(".github", context: instance_eval("binding"))
     end
@@ -117,12 +118,12 @@ def release
     desc "print_ruby_cc_version", "Output the cross compile ruby version needed for the gem. For internal usage", hide: true
     method_option "gemspec", type: "string", required: false, desc: "The gemspec to use. If the option is not passed, a gemspec file from the current working directory will be used."
     def print_ruby_cc_version
-      print compilation_task.ruby_cc_version
+      print(compilation_task.ruby_cc_version)
     end
 
     desc "normalized_platform", "The platform name for compilation purposes", hide: true
     def print_normalized_platform
-      print compilation_task.normalized_platform
+      print(compilation_task.normalized_platform)
     end
 
     private
@@ -143,7 +144,7 @@ def run_rake_tasks!(*tasks)
     def compilation_task
       @compilation_task ||= CompilationTasks.new(false)
     rescue GemspecError => e
-      print e.message
+      print(e.message)
 
       Kernel.exit(false)
     end
diff --git a/lib/cibuildgem/compilation_tasks.rb b/lib/cibuildgem/compilation_tasks.rb
@@ -10,7 +10,7 @@ class CompilationTasks
     attr_reader :gemspec, :native, :create_packaging_task, :extension_task
 
     def initialize(create_packaging_task = false, gemspec = nil)
-      @gemspec  = Bundler.load_gemspec(gemspec || find_gemspec)
+      @gemspec = Bundler.load_gemspec(gemspec || find_gemspec)
       verify_gemspec!
 
       @create_packaging_task = create_packaging_task
@@ -85,7 +85,7 @@ def disable_shared
           makefile_content.match(/LIBRUBYARG_SHARED = (.*)/) do |match|
             shared_flags = match[1].split(" ")
             shared_flags.reject! { |flag| flag == "-l$(RUBY_SO_NAME)" }
-            makefile_content.gsub!(/(LIBRUBYARG_SHARED = ).*/, "\\1#{shared_flags.join(' ')}")
+            makefile_content.gsub!(/(LIBRUBYARG_SHARED = ).*/, "\\1#{shared_flags.join(" ")}")
 
             File.write(task.name, makefile_content)
           end
diff --git a/lib/cibuildgem/ruby_series.rb b/lib/cibuildgem/ruby_series.rb
@@ -31,7 +31,7 @@ def versions_to_test_against(requirements)
       end.reverse
 
       selected_rubies.map do |version|
-        version.segments.tap(&:pop).join('.')
+        version.segments.tap(&:pop).join(".")
       end
     end
 
diff --git a/lib/cibuildgem/tasks/wrapper.rake b/lib/cibuildgem/tasks/wrapper.rake
@@ -22,7 +22,7 @@ end
 
 unless Rake::Task.task_defined?(:test)
   task(:test) do
-    raise(RuntimeError, "Don't know how to build task 'test'") unless Rake::Task.task_defined?(:spec)
+    raise("Don't know how to build task 'test'") unless Rake::Task.task_defined?(:spec)
 
     Rake::Task[:spec].invoke
   end
diff --git a/lib/cibuildgem/templates/.github/workflows/cibuildgem.yaml.tt b/lib/cibuildgem/templates/.github/workflows/cibuildgem.yaml.tt
@@ -13,15 +13,15 @@ jobs:
     name: "Cross compile the gem on different ruby versions"
     strategy:
       matrix:
-        os: <%= os %>
+        os: <%= @os %>
     runs-on: "${{ matrix.os }}"
     steps:
       - name: "Checkout code"
         uses: "actions/checkout@v5"
       - name: "Setup Ruby"
         uses: "ruby/setup-ruby@v1"
         with:
-          ruby-version: "<%= runtime_version_for_compilation %>"
+          ruby-version: "<%= @runtime_version_for_compilation %>"
           bundler-cache: true
           <%- if options['working-directory'] -%>
           working-directory: "<%= options['working-directory'] %>"
@@ -39,8 +39,8 @@ jobs:
     needs: compile
     strategy:
       matrix:
-        os: <%= os %>
-        rubies: <%= ruby_versions_for_testing %>
+        os: <%= @os %>
+        rubies: <%= @ruby_versions_for_testing %>
         type: ["cross", "native"]
     runs-on: "${{ matrix.os }}"
     steps:
@@ -70,13 +70,13 @@ jobs:
     needs: test
     strategy:
       matrix:
-        os: <%= os %>
+        os: <%= @os %>
     runs-on: "${{ matrix.os }}"
     steps:
       - name: "Setup Ruby"
         uses: "ruby/setup-ruby@v1"
         with:
-          ruby-version: "<%= latest_supported_ruby_version %>"
+          ruby-version: "<%= @latest_supported_ruby_version %>"
       - name: "Run cibuildgem"
         uses: "shopify/cibuildgem/.github/actions/cibuildgem@main"
         with:
diff --git a/test/cli_test.rb b/test/cli_test.rb
@@ -168,8 +168,6 @@ def test_when_cli_runs_in_project_with_no_gemspec
     end
 
     def test_when_cli_runs_in_project_with_no_native_extension
-      out = nil
-
       out, _ = capture_subprocess_io do
         raise_instead_of_exit do
           CLI.start(["print_ruby_cc_version"])
diff --git a/test/fixtures/dummy_gem/Gemfile b/test/fixtures/dummy_gem/Gemfile
@@ -5,5 +5,5 @@ source "https://rubygems.org"
 # Specify your gem's dependencies in dummy_gem.gemspec
 gemspec
 
-gem 'rake'
-gem 'minitest'
+gem "rake"
+gem "minitest"
diff --git a/test/fixtures/dummy_gem/dummy_gem.gemspec b/test/fixtures/dummy_gem/dummy_gem.gemspec
@@ -19,10 +19,10 @@ Gem::Specification.new do |spec|
   # Specify which files should be added to the gem when it is released.
   # The `git ls-files -z` loads the files in the RubyGem that have been added into git.
   gemspec = File.basename(__FILE__)
-  spec.files = IO.popen(%w[git ls-files -z], chdir: __dir__, err: IO::NULL) do |ls|
+  spec.files = IO.popen(["git", "ls-files", "-z"], chdir: __dir__, err: IO::NULL) do |ls|
     ls.readlines("\x0", chomp: true).reject do |f|
       (f == gemspec) ||
-        f.start_with?(*%w[bin/ Gemfile .gitignore test/])
+        f.start_with?("bin/", "Gemfile", ".gitignore", "test/")
     end
   end
   spec.bindir = "exe"
diff --git a/test/fixtures/dummy_gem/ext/hello_world/extconf.rb b/test/fixtures/dummy_gem/ext/hello_world/extconf.rb
@@ -2,6 +2,6 @@
 
 require "mkmf"
 
-$LDFLAGS << " -s -pipe" if RUBY_PLATFORM !~ /darwin/
+$LDFLAGS << " -s -pipe" if RUBY_PLATFORM !~ /darwin/ # rubocop:disable Style/GlobalVars
 
 create_makefile("hello_world")
diff --git a/test/fixtures/dummy_gem/lib/dummy_gem.rb b/test/fixtures/dummy_gem/lib/dummy_gem.rb
@@ -3,7 +3,7 @@
 require_relative "dummy_gem/version"
 
 begin
-  ruby_version = /(\d+\.\d+)/.match(::RUBY_VERSION)
+  ruby_version = /(\d+\.\d+)/.match(RUBY_VERSION)
 
   require "#{ruby_version}/hello_world"
 rescue LoadError
diff --git a/test/fixtures/dummy_gem/test/test_helper.rb b/test/fixtures/dummy_gem/test/test_helper.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-$LOAD_PATH.unshift File.expand_path("../lib", __dir__)
+$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
 require "dummy_gem"
 
 require "minitest/autorun"
diff --git a/test/fixtures/no_test_task_defined/foo.gemspec b/test/fixtures/no_test_task_defined/foo.gemspec
@@ -2,9 +2,9 @@
 
 Gem::Specification.new do |spec|
   spec.name = "foo"
-  spec.version = '0.0.1'
-  spec.authors = ["John Doe"]
-  spec.email = ["john@example.com"]
+  spec.version = "0.0.1"
+  spec.authors = ["Shopify"]
+  spec.email = ["rails@shopify.com"]
 
   spec.summary = "Not important"
   spec.description = "Not important"
diff --git a/test/fixtures/spec_task_defined/Rakefile b/test/fixtures/spec_task_defined/Rakefile
@@ -1,3 +1,5 @@
-task :spec do
+# frozen_string_literal: true
+
+task(:spec) do
   print "The spec task was called."
 end
diff --git a/test/fixtures/spec_task_defined/foo.gemspec b/test/fixtures/spec_task_defined/foo.gemspec
@@ -2,9 +2,9 @@
 
 Gem::Specification.new do |spec|
   spec.name = "foo"
-  spec.version = '0.0.1'
-  spec.authors = ["John Doe"]
-  spec.email = ["john@example.com"]
+  spec.version = "0.0.1"
+  spec.authors = ["Shopify"]
+  spec.email = ["rails@shopify.com"]
 
   spec.summary = "Not important"
   spec.description = "Not important"
diff --git a/test/fixtures/test_task_defined/Rakefile b/test/fixtures/test_task_defined/Rakefile
@@ -1,3 +1,5 @@
-task :test do
+# frozen_string_literal: true
+
+task(:test) do
   print "The test task was called."
 end
diff --git a/test/fixtures/test_task_defined/foo.gemspec b/test/fixtures/test_task_defined/foo.gemspec
@@ -2,7 +2,7 @@
 
 Gem::Specification.new do |spec|
   spec.name = "foo"
-  spec.version = '0.0.1'
+  spec.version = "0.0.1"
   spec.authors = ["John Doe"]
   spec.email = ["john@example.com"]
 
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-$LOAD_PATH.unshift File.expand_path("../lib", __dir__)
+$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
 require "cibuildgem"
 
 require "minitest/autorun"

Original file line number	Diff line number	Diff line change
`@@ -7,3 +7,4 @@ gemspec`
`7`	`7`
`8`	`8`	`gem "rake", "~> 13.0"`
`9`	`9`	`gem "minitest", "~> 5.16"`
	`10`	`+gem "rubocop-shopify"`