Skip to content

Commit

Permalink
Add Standard for Ruby formatting
Browse files Browse the repository at this point in the history
We had been using Hound CI and a custom RuboCop config to encourage
adhering to our style guide. In the last couple of years, Standard has
emerged and gained steam as a "set it and forget it" RuboCop
configuration for Ruby. Maintaining our own opinionated RuboCop
configuration is not something I care to do any longer. This change:

1. Adds Standard
2. Fixes all[^1] off the issues it detects
3. Adds Standard to CI
4. Removes our hound and RuboCop configuration

Adding Standard as an explicit CI step will require compliance for
passing builds, which is a departure from our previous setup of
only commenting via Hound. I think the time is right for this change
given the maturity of auto-fixers and integrations within various Ruby
workflows.

[^1]: We continue to allow the use of `eval` in our specs
  • Loading branch information
derekprior committed Jan 18, 2024
1 parent 104d888 commit 4440870
Show file tree
Hide file tree
Showing 36 changed files with 152 additions and 250 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@ on:
branches: "*"

jobs:
standard:
name: Lint with Standard
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Run standardrb
uses: standardrb/standard-ruby-action@f533e61f461ccb766b2d9c235abf59be02aea793
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

permissions:
checks: write
contents: read

build:
name: Ruby ${{ matrix.ruby }}, Rails ${{ matrix.rails }}

Expand Down
2 changes: 0 additions & 2 deletions .hound.yml

This file was deleted.

129 changes: 0 additions & 129 deletions .rubocop.yml

This file was deleted.

1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ agree to abide by our [code of conduct].
3. Run `rake` to verify that the tests pass against the version of Rails you are
running locally.
4. Make your change with new passing tests, following existing style.
5. Run `standardrb --fix` to ensure your code is formatted correctly.
5. Write a [good commit message], push your fork, and submit a pull request.
6. CI will run the test suite on all configured versions of Ruby and Rails.
Address any failures.
Expand Down
6 changes: 3 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ gemspec

rails_version = ENV.fetch("RAILS_VERSION", "6.1")

if rails_version == "master"
rails_constraint = { github: "rails/rails" }
rails_constraint = if rails_version == "master"
{github: "rails/rails"}
else
rails_constraint = "~> #{rails_version}.0"
"~> #{rails_version}.0"
end

gem "rails", rails_constraint
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

[![Build Status](https://github.com/scenic-views/scenic/actions/workflows/ci.yml/badge.svg)](https://github.com/scenic-views/scenic/actions/workflows/ci.yml)
[![Documentation Quality](http://inch-ci.org/github/scenic-views/scenic.svg?branch=master)](http://inch-ci.org/github/scenic-views/scenic)
[![Reviewed by Hound](https://img.shields.io/badge/Reviewed_by-Hound-8E64B0.svg)](https://houndci.com)

Scenic adds methods to `ActiveRecord::Migration` to create and manage database
views in Rails.
Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ RSpec::Core::RakeTask.new("spec:acceptance") do |task|
end

desc "Run the specs and acceptance tests"
task default: %w(spec spec:acceptance)
task default: %w[spec spec:acceptance]
27 changes: 27 additions & 0 deletions bin/standardrb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

#
# This file was generated by Bundler.
#
# The application 'standardrb' is installed as part of a gem, and
# this file is here to facilitate running it.
#

ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)

bundle_binstub = File.expand_path("bundle", __dir__)

if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
end
end

require "rubygems"
require "bundler/setup"

load Gem.bin_path("standard", "standardrb")
23 changes: 7 additions & 16 deletions lib/generators/scenic/model/model_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def invoke_rails_model_generator
[file_path.singularize],
options.merge(
fixture_replacement: false,
migration: false,
migration: false
)
end

Expand All @@ -34,23 +34,14 @@ def invoke_view_generator
private

def evaluate_template(source)
source = File.expand_path(find_in_source_paths(source.to_s))
source = File.expand_path(find_in_source_paths(source.to_s))
context = instance_eval("binding", __FILE__, __LINE__)

if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+
erb = ERB.new(
::File.binread(source),
trim_mode: "-",
eoutvar: "@output_buffer",
)
else
erb = ERB.new(
::File.binread(source),
nil,
"-",
"@output_buffer",
)
end
erb = ERB.new(
::File.binread(source),
trim_mode: "-",
eoutvar: "@output_buffer"
)

erb.result(context)
end
Expand Down
10 changes: 5 additions & 5 deletions lib/generators/scenic/view/view_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ def create_migration_file
if creating_new_view? || destroying_initial_view?
migration_template(
"db/migrate/create_view.erb",
"db/migrate/create_#{plural_file_name}.rb",
"db/migrate/create_#{plural_file_name}.rb"
)
else
migration_template(
"db/migrate/update_view.erb",
"db/migrate/update_#{plural_file_name}_to_version_#{version}.rb",
"db/migrate/update_#{plural_file_name}_to_version_#{version}.rb"
)
end
end
Expand All @@ -56,7 +56,7 @@ def version

def migration_class_name
if creating_new_view?
"Create#{class_name.tr('.', '').pluralize}"
"Create#{class_name.tr(".", "").pluralize}"
else
"Update#{class_name.pluralize}ToVersion#{version}"
end
Expand All @@ -73,7 +73,7 @@ def activerecord_migration_class

private

alias singular_name file_name
alias_method :singular_name, :file_name

def file_name
super.tr(".", "_")
Expand Down Expand Up @@ -113,7 +113,7 @@ def formatted_plural_name

def create_view_options
if materialized?
", materialized: #{no_data? ? '{ no_data: true }' : true}"
", materialized: #{no_data? ? "{ no_data: true }" : true}"
else
""
end
Expand Down
6 changes: 3 additions & 3 deletions lib/scenic/adapters/postgres.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ def create_materialized_view(name, sql_definition, no_data: false)

execute <<-SQL
CREATE MATERIALIZED VIEW #{quote_table_name(name)} AS
#{sql_definition.rstrip.chomp(';')}
#{'WITH NO DATA' if no_data};
#{sql_definition.rstrip.chomp(";")}
#{"WITH NO DATA" if no_data};
SQL
end

Expand Down Expand Up @@ -271,7 +271,7 @@ def refresh_dependencies_for(name, concurrently: false)
name,
self,
connection,
concurrently: concurrently,
concurrently: concurrently
)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/scenic/adapters/postgres/indexes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def index_from_database(result)
Scenic::Index.new(
object_name: result["object_name"],
index_name: result["index_name"],
definition: result["definition"],
definition: result["definition"]
)
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/scenic/adapters/postgres/refresh_dependencies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def call
dependencies.each do |dependency|
adapter.refresh_materialized_view(
dependency,
concurrently: concurrently,
concurrently: concurrently
)
end
end
Expand Down Expand Up @@ -103,8 +103,8 @@ def tsort(hash)
ORDER BY class_for_rewrite.relname;
SQL

private_constant "DependencyParser"
private_constant "DEPENDENCY_SQL"
private_constant :DependencyParser
private_constant :DEPENDENCY_SQL

def dependencies
raw_dependency_info = connection.select_rows(DEPENDENCY_SQL)
Expand Down
10 changes: 5 additions & 5 deletions lib/scenic/adapters/postgres/views.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,21 @@ def views_from_postgres
def to_scenic_view(result)
namespace, viewname = result.values_at "namespace", "viewname"

if namespace != "public"
namespaced_viewname = "#{pg_identifier(namespace)}.#{pg_identifier(viewname)}"
namespaced_viewname = if namespace != "public"
"#{pg_identifier(namespace)}.#{pg_identifier(viewname)}"
else
namespaced_viewname = pg_identifier(viewname)
pg_identifier(viewname)
end

Scenic::View.new(
name: namespaced_viewname,
definition: result["definition"].strip,
materialized: result["kind"] == "m",
materialized: result["kind"] == "m"
)
end

def pg_identifier(name)
return name if name =~ /^[a-zA-Z_][a-zA-Z0-9_]*$/
return name if /^[a-zA-Z_][a-zA-Z0-9_]*$/.match?(name)

pgconn.quote_ident(name)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/scenic/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def version
attr_reader :name

def filename
"#{UnaffixedName.for(name).tr('.', '_')}_v#{version}.sql"
"#{UnaffixedName.for(name).tr(".", "_")}_v#{version}.sql"
end
end
end
Loading

0 comments on commit 4440870

Please sign in to comment.