BuoySoftware / guides

Documentation for Buoy Software
2 stars 0 forks source link

Use with_fixed_indentation for ArgumentAlignment #70

Closed calebhearth closed 1 year ago

calebhearth commented 1 year ago

Rather than wasting a bunch of space on indenting with the first argument, use fixed indentation. This is really nice given our line length of 80 as in many cases with_first_argument was leading to awkward line breaking.

Note that this was originally set in https://github.com/BuoySoftware/guides/pull/66 with the goal of reducing ERB violations, but that we separately excluded erb files from this rule so this change wasn't really needed to match our current style.

Here's an example diff from running rubocop -A --only Layout/ArgumentAlignment in Buoy (it's abbreviated):

diff --git a/Gemfile b/Gemfile
index 0cf453384..6f9e9a612 100644
--- a/Gemfile
+++ b/Gemfile
@@ -126,8 +126,8 @@ group :test do
   gem "bunny-mock"
   gem "capybara"
   gem "capybara_accessible_selectors",
-      git: "https://github.com/citizensadvice/capybara_accessible_selectors",
-      branch: "main"
+    git: "https://github.com/citizensadvice/capybara_accessible_selectors",
+    branch: "main"
   gem "capybara-chromedriver-logger"
   gem "capybara_discoball"
   gem "capybara-experience"
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index c05a4e33c..21d376f38 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -8,7 +8,7 @@ def admin_resources
   def launch_darkly_meta_tag
     if ENV.key?("LAUNCH_DARKLY_CLIENT_ID")
       tag.meta(name: "launch-darkly-client-id",
-               content: ENV.fetch("LAUNCH_DARKLY_CLIENT_ID"))
+        content: ENV.fetch("LAUNCH_DARKLY_CLIENT_ID"))
     end
   end

diff --git a/app/models/donor_document/confirmation.rb b/app/models/donor_document/confirmation.rb
index 5022c35d5..2c59bd17a 100644
--- a/app/models/donor_document/confirmation.rb
+++ b/app/models/donor_document/confirmation.rb
@@ -3,7 +3,7 @@ class Confirmation
     include ActiveModel::Model

     attr_accessor :donor_document, :employee, :confirmed_signed,
-                  :ignore_initiator
+      :ignore_initiator

     delegate :donor, to: :donor_document

diff --git a/app/models/facility.rb b/app/models/facility.rb
index 9bf5ee3dd..1b29b7d2e 100644
--- a/app/models/facility.rb
+++ b/app/models/facility.rb
@@ -31,13 +31,13 @@ class Facility < ApplicationRecord
   has_many :screenings, through: :donations
   has_many :setups, through: :donations
   has_many :units,
-           through: :visits, dependent: :destroy, inverse_of: :facility
+    through: :visits, dependent: :destroy, inverse_of: :facility

   has_one :center_location,
-          -> { active.center },
-          class_name: "Location",
-          dependent: :destroy,
-          inverse_of: :facility
+    -> { active.center },
+    class_name: "Location",
+    dependent: :destroy,
+    inverse_of: :facility
   has_one :hours_of_operation, dependent: :destroy

   enum :status, {
@@ -49,14 +49,14 @@ class Facility < ApplicationRecord
   validates :city, presence: true
   validates :name, presence: true
   validates :nddr_cdcs_site_code,
-            uniqueness: true,
-            allow_blank: true,
-            length: { maximum: 5 }
+    uniqueness: true,
+    allow_blank: true,
+    length: { maximum: 5 }
   validates :opens_on, presence: true
   validates :prefix,
-            uniqueness: true,
-            presence: true,
-            length: { maximum: 2, allow_blank: true }
+    uniqueness: true,
+    presence: true,
+    length: { maximum: 2, allow_blank: true }
   validates :state_or_province, presence: true
   validates :zipcode, presence: true
arzezak commented 1 year ago

Do you have an example of how this looks with ERB files?

calebhearth commented 1 year ago

Good point, rubocop -A doesn't touch ERB files so I didn't have a diff, but this does re-introduce the problem you originally tried to fix. I propose that we disable this rule as well for ERB. The diff when I manually fixed it looked like this:

  <%= image_tag("slug.png",
-   alt: "it's slimy"
- ) %>
+       alt: "it's slimy"
+     ) %>
arzezak commented 1 year ago

Good point, rubocop -A doesn't touch ERB files so I didn't have a diff, but this does re-introduce the problem you originally tried to fix. I propose that we disable this rule as well for ERB. The diff when I manually fixed it looked like this:

  <%= image_tag("slug.png",
-   alt: "it's slimy"
- ) %>
+       alt: "it's slimy"
+     ) %>

🤣 I thought so! That was the problem when I initially looked at this. For the record, I favor disabling it on ERB files.