From 5f66afb5db12dccf8b682339a0165714a35f2645 Mon Sep 17 00:00:00 2001 From: John Boyes Date: Sat, 22 Aug 2020 19:31:12 +0700 Subject: [PATCH] Update Rubocop checking (#106) * Fix problem with RuboCop checks We were using Hound for our RuboCop checks, but it wasn't working properly for some reason. Switching to [Reviewdog's GitHub Action][1]. [1]: https://github.com/reviewdog/action-rubocop * Rename Reviewdog GitHub Actions The previous naming format led to our Reviewdog actions appearing with an incorrect name in the GitHub Actions UI. * Use the RuboCop default line length of 120 chars [RuboCop changed the default from 80 chars to 120][1] in May 2020. [1]: https://github.com/rubocop-hq/rubocop/pull/7952 * Add an empty line after guard clauses As prompted by RuboCop. * Rename variable as prompted by RuboCop * Rename variable as prompted by RuboCop * Remove unused block argument as per RuboCop * Move private declaration to outside method As per RuboCop * Remove obsolete RuboCop exclusion * Enable all new RuboCop cops The more RuboCop checks, the better :-) * Fix new RuboCop errors --- .github/workflows/reviewdog.yml | 15 +++++++++++++-- .hound.yml | 3 --- .rubocop.yml | 11 ++++------- config.ru | 2 +- google_sheet.rb | 26 ++++++++++++++------------ hcmoney.rb | 3 +++ hot_custard_payments.rb | 5 +++-- populate_datastore.rb | 5 ++++- 8 files changed, 42 insertions(+), 28 deletions(-) delete mode 100644 .hound.yml diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 171da36..eeebeef 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -6,7 +6,7 @@ on: # yamllint disable-line rule:truthy jobs: hadolint: - name: runner / hadolint + name: hadolint runs-on: ubuntu-20.04 steps: - name: Check out code @@ -18,7 +18,7 @@ jobs: reporter: github-check yamllint: - name: runner / yamllint + name: yamllint runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2.3.2 @@ -27,3 +27,14 @@ jobs: with: github_token: ${{ secrets.github_token }} reporter: github-check + + rubocop: + name: rubocop + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v2.3.2 + - name: rubocop + uses: reviewdog/action-rubocop@v1.5.0 + with: + github_token: ${{ secrets.github_token }} + reporter: github-check diff --git a/.hound.yml b/.hound.yml deleted file mode 100644 index fe16505..0000000 --- a/.hound.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -ruby: - config_file: .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml index 194ab1e..77a21e3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,14 +1,11 @@ --- -Layout/LineLength: - Max: 100 + +AllCops: + NewCops: enable + Style/FrozenStringLiteralComment: Enabled: false -Style/FormatStringToken: - # Thefalse positives in the full journey spec should be fixed in a new version of RuboCop soon - see # - Exclude: - - spec/full_journey_spec.rb - Metrics/BlockLength: Exclude: - 'Rakefile' diff --git a/config.ru b/config.ru index 9f537f7..77745bc 100644 --- a/config.ru +++ b/config.ru @@ -14,5 +14,5 @@ OmniAuth.config.on_failure = proc do |env| OmniAuth::FailureEndpoint.new(env).redirect_to_failure end -require './hot_custard_payments.rb' +require './hot_custard_payments' run HotCustardApp diff --git a/google_sheet.rb b/google_sheet.rb index f00ed43..3325001 100644 --- a/google_sheet.rb +++ b/google_sheet.rb @@ -20,14 +20,6 @@ def range(spreadsheet_key, range) end end - private def boundaries(a1notation_range) - Hash.new({}).tap do |the_boundaries| - a1_notation = a1notation_range.split('!').last.split(':') - the_boundaries[:start_column], the_boundaries[:start_row] = a1_notation.first.scan(/\D+|\d+/) - the_boundaries[:end_column], the_boundaries[:end_row] = a1_notation.last.split(':').last.scan(/\D+|\d+/) - end - end - def spreadsheet(spreadsheet_key) # We use a backoff here to avoid hitting the Google Sheets API rate limit per 100 seconds exponential_backoff do @@ -35,16 +27,16 @@ def spreadsheet(spreadsheet_key) end end - def exponential_wait_time(n) - 2**n.tap { |wait_time| puts "wait time: #{wait_time}s" } + def exponential_wait_time(counter) + 2**counter.tap { |wait_time| puts "wait time: #{wait_time}s" } end # rubocop:disable Style/RescueStandardError def exponential_backoff (0..10).each do |n| return yield - rescue => error - puts error.inspect + rescue => e + puts e.inspect sleep(exponential_wait_time(n)) next end @@ -76,5 +68,15 @@ def to_hash_array(cells_with_header_row) def title(spreadsheet_key) spreadsheet(spreadsheet_key).properties.title end + + private + + def boundaries(a1notation_range) + Hash.new({}).tap do |the_boundaries| + a1_notation = a1notation_range.split('!').last.split(':') + the_boundaries[:start_column], the_boundaries[:start_row] = a1_notation.first.scan(/\D+|\d+/) + the_boundaries[:end_column], the_boundaries[:end_row] = a1_notation.last.split(':').last.scan(/\D+|\d+/) + end + end end end diff --git a/hcmoney.rb b/hcmoney.rb index 95b9b9e..ae37076 100644 --- a/hcmoney.rb +++ b/hcmoney.rb @@ -22,6 +22,7 @@ class HCMoney def self.amount_that_can_be_credited(creditor_balance:, hot_custard_balance:) return [hot_custard_balance, -creditor_balance].max if hot_custard_balance.negative? + case hot_custard_balance <=> creditor_balance when -1 then hot_custard_balance when 0 then hot_custard_balance @@ -59,6 +60,7 @@ def markup_to_cover_transfer_fees_and_rate_fluctuation(markup_percentage) def worth_showing? return false if blank? @monetary_string + (to_i >= 1) || (to_i <= -1) end @@ -89,6 +91,7 @@ def /(other) def <=>(other) return nil unless other.is_a?(HCMoney) + @money <=> other.money end diff --git a/hot_custard_payments.rb b/hot_custard_payments.rb index b72d80e..01865df 100644 --- a/hot_custard_payments.rb +++ b/hot_custard_payments.rb @@ -24,13 +24,14 @@ class HotCustardApp < Sinatra::Base set(:role) { |role| condition { halt 403 if (role == :financial_admin) && !financial_admin? } } before do - pass if request.path_info.match? %r{^\/auth\/|^\/privacy$} + pass if request.path_info.match? %r{^/auth/|^/privacy$} redirect to('/auth/facebook') unless current_user end def individual_balances_for(username) datastore_balances = user_datastore["balance:#{username}"] return {} unless datastore_balances + JSON.parse(datastore_balances).select { |_key, value| HCMoney.new(value).worth_showing? } end @@ -92,7 +93,7 @@ def creditor_item_amounts(creditor_balance, hot_custard_balance) def balances_for(person) string_balances = JSON.parse user_datastore["balance:#{person}"] - string_balances.map { |item, amount| [item, HCMoney.new(amount)] }.to_h + string_balances.transform_values { |amount| HCMoney.new(amount) } end def credits_for(creditor) diff --git a/populate_datastore.rb b/populate_datastore.rb index 9f4b7b1..8cf0097 100644 --- a/populate_datastore.rb +++ b/populate_datastore.rb @@ -12,7 +12,7 @@ DATASTORE = Redis.new(url: ENV['REDIS_URL']) def check_worksheets - spreadsheet_keys.reverse.take(20).each_with_index { |key, i| check_worksheet(key) } + spreadsheet_keys.reverse.take(20).each { |key| check_worksheet(key) } end def check_worksheet(spreadsheet_key) @@ -20,6 +20,7 @@ def check_worksheet(spreadsheet_key) start_row = people_with_costs[:boundaries][:start_row] full_people_row = GoogleSheet.range(spreadsheet_key, "#{start_row}:#{start_row}")[:values].first raise invalid_spreadsheet_message(spreadsheet_key) unless full_people_row_valid? full_people_row + puts "Successfully validated spreadsheet #{spreadsheet_key}" end @@ -31,9 +32,11 @@ def full_people_row_valid?(full_people_row) sliced = full_people_row.slice_after('').to_a return false unless sliced.size == 5 return false unless sliced[1..3].all? { |a| a == [''] } + sliced[0].pop return false unless sliced[0] == sliced[4] return false unless sliced[0] == sliced[0].uniq + true end