From 9a458a7178dede05d043b923a80904963dee298e Mon Sep 17 00:00:00 2001 From: Daniel Ma Date: Thu, 4 May 2017 13:21:12 -0700 Subject: [PATCH 1/2] rubocop -a --- Gemfile | 4 +++- Gemfile.lock | 14 ++++++++++++++ Rakefile | 1 + lib/review_bot.rb | 11 ++++++----- lib/review_bot/extensions.rb | 1 + lib/review_bot/hour_of_day.rb | 5 +++-- lib/review_bot/pull_request.rb | 23 ++++++++++++----------- lib/review_bot/reminder.rb | 17 +++++++++-------- lib/review_bot/reviewer.rb | 3 ++- review.rake | 7 ++++--- 10 files changed, 55 insertions(+), 31 deletions(-) diff --git a/Gemfile b/Gemfile index f625be1..3ca6c6e 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,8 @@ +# frozen_string_literal: true source 'https://rubygems.org' +gem 'awesome_print' gem 'github_api' gem 'rest-client' +gem 'rubocop' gem 'timezone' -gem 'awesome_print' diff --git a/Gemfile.lock b/Gemfile.lock index 902f336..87ca14d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,6 +2,7 @@ GEM remote: https://rubygems.org/ specs: addressable (2.4.0) + ast (2.3.0) awesome_print (1.7.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) @@ -31,16 +32,28 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) + parser (2.4.0.0) + ast (~> 2.2) + powerpack (0.1.1) rack (1.6.4) + rainbow (2.2.1) rest-client (1.8.0) http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 3.0) netrc (~> 0.7) + rubocop (0.47.1) + parser (>= 2.3.3.1, < 3.0) + powerpack (~> 0.1) + rainbow (>= 1.99.1, < 3.0) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) + ruby-progressbar (1.8.1) thread_safe (0.3.5) timezone (0.99.2) unf (0.1.4) unf_ext unf_ext (0.0.7.2) + unicode-display_width (1.1.3) PLATFORMS ruby @@ -49,6 +62,7 @@ DEPENDENCIES awesome_print github_api rest-client + rubocop timezone BUNDLED WITH diff --git a/Rakefile b/Rakefile index b9b6bbf..841b22e 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'bundler' Bundler.setup diff --git a/lib/review_bot.rb b/lib/review_bot.rb index 89c20c2..1308d0b 100644 --- a/lib/review_bot.rb +++ b/lib/review_bot.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'json' require 'github_api' require 'rest-client' @@ -5,11 +6,11 @@ require 'time' require 'awesome_print' -require_relative "review_bot/hour_of_day" -require_relative "review_bot/pull_request" -require_relative "review_bot/extensions" -require_relative "review_bot/reviewer" -require_relative "review_bot/reminder" +require_relative 'review_bot/hour_of_day' +require_relative 'review_bot/pull_request' +require_relative 'review_bot/extensions' +require_relative 'review_bot/reviewer' +require_relative 'review_bot/reminder' module ReviewBot end diff --git a/lib/review_bot/extensions.rb b/lib/review_bot/extensions.rb index f08e6db..691ddd2 100644 --- a/lib/review_bot/extensions.rb +++ b/lib/review_bot/extensions.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Time def round_to(interval) self.class.at((to_f / interval).round * interval).utc diff --git a/lib/review_bot/hour_of_day.rb b/lib/review_bot/hour_of_day.rb index 1801185..66ff8a9 100644 --- a/lib/review_bot/hour_of_day.rb +++ b/lib/review_bot/hour_of_day.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module ReviewBot class HourOfDay # 8am to 5pm are work hours, so we only count the hour if it's inside that range @@ -14,7 +15,7 @@ def self.work_hours_between(start_time, end_time, timezone) while rounded_start_time < rounded_end_time rounded_start_time += ONE_HOUR - work_hours += 1 if new(timezone.utc_to_local rounded_start_time).work_hour? + work_hours += 1 if new(timezone.utc_to_local(rounded_start_time)).work_hour? end work_hours @@ -31,7 +32,7 @@ def work_hour? end def inspect - "HourOfDay(#{rounded_time.strftime("%a-%I%P")} work_hour: #{work_hour?})" + "HourOfDay(#{rounded_time.strftime('%a-%I%P')} work_hour: #{work_hour?})" end end end diff --git a/lib/review_bot/pull_request.rb b/lib/review_bot/pull_request.rb index 9cfd7ea..0d4b649 100644 --- a/lib/review_bot/pull_request.rb +++ b/lib/review_bot/pull_request.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module ReviewBot class PullRequest < SimpleDelegator def needs_review? @@ -9,9 +10,9 @@ def needs_first_review? end def reviewers - [comments_from_other_humans, reviews_from_other_humans].flatten. - map { |i| i['user']['login'] }. - uniq + [comments_from_other_humans, reviews_from_other_humans].flatten + .map { |i| i['user']['login'] } + .uniq end def last_touched_at @@ -30,9 +31,9 @@ def inspect "last touched: #{last_touched_at}", "review_in_progress: #{review_in_progress?}", "needs_review: #{needs_review?}", - "url: #{html_url}", - ].join(", ") + - " )" + "url: #{html_url}" + ].join(', ') + + ' )' end private @@ -57,9 +58,9 @@ def review_in_progress? end def last_touch - @last_touch ||= [comments_from_other_humans, reviews_from_other_humans].flatten. - sort_by { |comment_or_review| comment_or_review['created_at'] || comment_or_review['submitted_at'] }. - last + @last_touch ||= [comments_from_other_humans, reviews_from_other_humans].flatten + .sort_by { |comment_or_review| comment_or_review['created_at'] || comment_or_review['submitted_at'] } + .last end def repo_owner @@ -92,13 +93,13 @@ def comments_from_other_humans def reviews # github_api doesn't support this yet - @reviews ||= ( + @reviews ||= begin conn = Faraday.new( url: 'https://api.github.com', headers: { Accept: 'application/vnd.github.black-cat-preview+json' } ) JSON.parse(conn.get("/repos/#{repo_owner}/#{repo_name}/pulls/#{number}/reviews?access_token=#{ENV['GH_AUTH_TOKEN']}").body) - ) + end end def reviews_from_humans diff --git a/lib/review_bot/reminder.rb b/lib/review_bot/reminder.rb index 0923cef..f39a800 100644 --- a/lib/review_bot/reminder.rb +++ b/lib/review_bot/reminder.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module ReviewBot GH = Github.new(oauth_token: ENV['GH_AUTH_TOKEN']) @@ -18,10 +19,10 @@ def message message + notifications.map do |notification| [ "#{notification.pull_request.html_url} needs a", - notification.pull_request.needs_first_review? ? "first review" : "second review", - "from", - notification.suggested_reviewers.map { |r| ":#{r.slack}:" }.join(" "), - ].join(" ") + notification.pull_request.needs_first_review? ? 'first review' : 'second review', + 'from', + notification.suggested_reviewers.map { |r| ":#{r.slack}:" }.join(' ') + ].join(' ') end.join("\n\n") end @@ -49,9 +50,9 @@ def potential_notifications next if work_hours_since_last_touch < app_config['hours_to_review'] - suggested_reviewers = potential_reviewers. - select(&:work_hour?). - reject do |reviewer| + suggested_reviewers = potential_reviewers + .select(&:work_hour?) + .reject do |reviewer| pull.reviewers.include?(reviewer['github']) end @@ -60,7 +61,7 @@ def potential_notifications OpenStruct.new( pull_request: pull, suggested_reviewers: suggested_reviewers, - work_hours_since_last_touch: work_hours_since_last_touch, + work_hours_since_last_touch: work_hours_since_last_touch ) end end diff --git a/lib/review_bot/reviewer.rb b/lib/review_bot/reviewer.rb index 3921785..e633313 100644 --- a/lib/review_bot/reviewer.rb +++ b/lib/review_bot/reviewer.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module ReviewBot class Reviewer < OpenStruct def work_hours_between(start_time, end_time) @@ -9,7 +10,7 @@ def timezone end def work_hour? - HourOfDay.new(timezone.utc_to_local Time.now.utc).work_hour? + HourOfDay.new(timezone.utc_to_local(Time.now.utc)).work_hour? end end end diff --git a/review.rake b/review.rake index f644fdb..ffa5678 100644 --- a/review.rake +++ b/review.rake @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'json' CONFIG = JSON.parse(ENV['CONFIG']) @@ -9,12 +10,12 @@ require_relative 'lib/review_bot' desc 'Send reminders to team members to review PRs' task :remind, [:mode] do |_t, args| - dry_run = args[:mode] == "dry" + dry_run = args[:mode] == 'dry' puts "-- DRY RUN --\n\n" if dry_run CONFIG.each do |app, app_config| - owner, repo = app.split("/") + owner, repo = app.split('/') room = app_config['room'] puts "#{owner}/#{repo}" @@ -38,7 +39,7 @@ task :remind, [:mode] do |_t, args| channel: room, text: message, icon_emoji: SLACK_BOT_ICON, - username: SLACK_BOT_NAME, + username: SLACK_BOT_NAME ) end end From 23405832b83293701f4fd7e424f7c0e0dc1d6bab Mon Sep 17 00:00:00 2001 From: Daniel Ma Date: Thu, 4 May 2017 13:33:30 -0700 Subject: [PATCH 2/2] change: add all faces even if it's not a work hour This improves the visual experience of looking through recent messages and shows the reader who needs to review, even if their work hours aren't being counted at the time --- lib/review_bot/reminder.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/review_bot/reminder.rb b/lib/review_bot/reminder.rb index f39a800..aa77a58 100644 --- a/lib/review_bot/reminder.rb +++ b/lib/review_bot/reminder.rb @@ -50,9 +50,7 @@ def potential_notifications next if work_hours_since_last_touch < app_config['hours_to_review'] - suggested_reviewers = potential_reviewers - .select(&:work_hour?) - .reject do |reviewer| + suggested_reviewers = potential_reviewers.reject do |reviewer| pull.reviewers.include?(reviewer['github']) end