Skip to content

Commit

Permalink
Use github search to power pull request search
Browse files Browse the repository at this point in the history
Currently, the seal loops through all repos and fetches all pull
requests. We could replace this with the issue search:
https://developer.github.com/v3/search/#search-issues

This will return all open PRs in all alphagov repos. It uses the same
search syntax as github.com.

This means 1) we can get rid of the repos in the configuration, 2) new
repos are added automatically, 3) we save a bunch of requests.

Closes: #52
  • Loading branch information
tijmenb committed Dec 16, 2015
1 parent f363984 commit 12f37c6
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 191 deletions.
4 changes: 0 additions & 4 deletions app.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@
"GITHUB_TOKEN": {
"description": "Your Github token (obtainable at https://github.com/settings/tokens)"
},
"GITHUB_REPOS": {
"description": "Comma separated list of repos to check",
"required": false
},
"GITHUB_MEMBERS": {
"description": "Comma separated list of github members",
"required": false
Expand Down
132 changes: 0 additions & 132 deletions config/alphagov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,6 @@ core-formats:
- fofr
- jamiecobbett

repos:
- Accessible-Media-Player
- asset-manager
- content-register
- content-store
- deprecated_columns
- external-link-tracker
- feedback
- frontend
- gds_zendesk
- government-frontend
- govuk_admin_template
- govuk_content_api
- govuk-content-schemas
- govuk_content_models
- govuk_need_api
- info-frontend
- maslow
- metadata-api
- panopticon
- publisher
- release
- router-api
- short-url-manager
- signonotron2
- specialist-frontend
- specialist-publisher
- static
- support
- support-api
- travel-advice-publisher
- url-arbiter
- whitehall

channel:
"#core-formats"

Expand All @@ -62,37 +28,6 @@ custom:
- h-lame
- davidbasalla

repos:
- business-support-api
- calendars
- contacts-admin
- contacts-frontend
- content-register
- content-store
- courts-api
- courts-frontend
- govuk-content-schemas
- hmrc-manuals-api
- imminence
- licence-finder
- manuals-frontend
- mapit
- optic14n
- pre-transition-stats
- publishing-api
- side-by-side-browser
- smart-answers
- specialist-publisher
- static
- trade-tariff-admin
- trade-tariff-backend
- trade-tariff-frontend
- transition
- transition-config
- transition-stats
- url-arbiter
- whitehall

channel:
"#custom"

Expand All @@ -110,45 +45,6 @@ finding-things:
- rboulton
- stephen-richards
- tijmenb

repos:
- collections
- collections-publisher
- content-store
- content-tagger
- email-alert-api
- email-alert-monitor
- finder-frontend
- govdelivery-email-templates
- government-service-design-manual
- govuk_content_api
- govuk_content_models
- govuk_message_queue_consumer
- govuk_migration_plan_public
- govuk_mirror-puppet
- govuk_prototype_kit
- govuk-content-best-practices
- govuk-content-schema-test-helpers
- govuk-content-schemas
- panopticon
- payment-prototype-options
- policy-publisher
- prototyping
- publishing-api
- recommended-links
- release
- router
- rummager
- search-admin
- search-performance-dashboard
- smokey
- specialist-publisher
- static
- styleguides
- support
- support-api
- tagging-migration-verifier
- whitehall

channel:
"#finding-things"
Expand All @@ -167,34 +63,6 @@ testing:
- fofr
- jamiecobbett

repos:
- Accessible-Media-Player
- asset-manager
- content-register
- content-store
- external-link-tracker
- feedback
- frontend
- government-frontend
- govuk_content_api
- govuk-content-schemas
- info-frontend
- maslow
- metadata-api
- panopticon
- publisher
- release
- short-url-manager
- signonotron2
- specialist-frontend
- specialist-publisher
- static
- support
- support-api
- travel-advice-publisher
- url-arbiter
- whitehall

channel:
"#testing"

Expand Down
25 changes: 0 additions & 25 deletions config/envato.yml
Original file line number Diff line number Diff line change
@@ -1,35 +1,10 @@
envato-market:
channel: '#marketplacedev'
members: []
repos:
- db_backup
- docs
- envato.com
- front-end-coding-test
- market-dev-expectations
- marketplace
- marketplace-puppet
- new-coding-test
- ops-coding-test
- our-boxen
- webuild.envato.com
use_labels: true
exclude_labels:
- wip
discovery:
channel: '#discovery-notify'
members: []
repos:
- discovery
- solid_octane
- solid_octane_service
- dewey
- discovery-scheduler
- recommender-api
- api-gateway
- build.envato.com
- search_public_api
- discovery_users
- ami-builder
- discovery-docker
use_labels: false
42 changes: 23 additions & 19 deletions lib/github_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,48 @@
class GithubFetcher
ORGANISATION ||= ENV['SEAL_ORGANISATION']

attr_accessor :people, :repos
attr_accessor :people

def initialize(team_members_accounts, team_repos, use_labels, exclude_labels, exclude_titles)
def initialize(team_members_accounts, use_labels, exclude_labels, exclude_titles)
@github = Octokit::Client.new(:access_token => ENV['GITHUB_TOKEN'])
@github.user.login
Octokit.auto_paginate = true
@people = team_members_accounts
@repos = team_repos.sort!
@pull_requests = {}
@use_labels = use_labels
@exclude_labels = exclude_labels.map(&:downcase).uniq if exclude_labels
@exclude_titles = exclude_titles.map(&:downcase).uniq if exclude_titles
@labels = {}
end

def list_pull_requests
@repos.each do |repo|
response = @github.pull_requests("#{ORGANISATION}/#{repo}", state: "open")
response.reject { |pr| hidden?(pr, repo) }.each do |pull_request|
@pull_requests[pull_request.title] = {}.tap do |pr|
pr['title'] = pull_request.title
pr['link'] = pull_request.html_url
pr['author'] = pull_request.user.login
pr['repo'] = repo
pr['comments_count'] = count_comments(pull_request, repo)
pr['thumbs_up'] = count_thumbs_up(pull_request, repo)
pr['updated'] = Date.parse(pull_request.updated_at.to_s)
pr['labels'] = labels(pull_request, repo)
end
end
pull_requests_from_github.each_with_object({}) do |pull_request, pull_requests|
repo_name = pull_request.html_url.split("/")[4]
next if hidden?(pull_request, repo_name)
pull_requests[pull_request.title] = present_pull_request(pull_request, repo_name)
end
@pull_requests
end

private

attr_reader :use_labels, :exclude_labels, :exclude_titles

def present_pull_request(pull_request, repo_name)
pr = {}
pr['title'] = pull_request.title
pr['link'] = pull_request.html_url
pr['author'] = pull_request.user.login
pr['repo'] = repo_name
pr['comments_count'] = count_comments(pull_request, repo_name)
pr['thumbs_up'] = count_thumbs_up(pull_request, repo_name)
pr['updated'] = Date.parse(pull_request.updated_at.to_s)
pr['labels'] = labels(pull_request, repo_name)
pr
end

def pull_requests_from_github
@github.search_issues("is:pr state:open user:#{ORGANISATION}").items
end

def person_subscribed?(pull_request)
people.empty? || people.include?("#{pull_request.user.login}")
end
Expand Down
3 changes: 0 additions & 3 deletions lib/seal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,17 @@ def pull_requests(team)
config = team_config(team)
if config
members = config['members']
repos = config['repos']
use_labels = config['use_labels']
exclude_labels = config['exclude_labels']
exclude_titles = config['exclude_titles']
else
members = ENV['GITHUB_MEMBERS'] ? ENV['GITHUB_MEMBERS'].split(',') : []
repos = ENV['GITHUB_REPOS'] ? ENV['GITHUB_REPOS'].split(',') : []
use_labels = ENV['GITHUB_USE_LABELS'] ? ENV['GITHUB_USE_LABELS'].split(',') : nil
exclude_labels = ENV['GITHUB_EXCLUDE_LABELS'] ? ENV['GITHUB_EXCLUDE_LABELS'].split(',') : nil
exclude_titles = ENV['GITHUB_EXCLUDE_TITLES'] ? ENV['GITHUB_EXCLUDE_TITLES'].split(',') : nil
end

git = GithubFetcher.new(members,
repos,
use_labels,
exclude_labels,
exclude_titles
Expand Down
4 changes: 1 addition & 3 deletions spec/github_fetcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
describe 'GithubFetcher' do
subject(:github_fetcher) do
GithubFetcher.new(team_members_accounts,
team_repos,
use_labels,
exclude_labels,
exclude_titles
Expand Down Expand Up @@ -58,7 +57,6 @@
let(:exclude_labels) { nil }
let(:exclude_titles) { nil }
let(:team_members_accounts) { %w(binaryberry boffbowsh jackscotti tekin elliotcm tommyp mattbostock) }
let(:team_repos) { %w(whitehall) }
let(:pull_2266) do
double(Sawyer::Resource,
user: double(Sawyer::Resource, login: 'mattbostock'),
Expand Down Expand Up @@ -97,7 +95,7 @@
before do
expect(Octokit::Client).to receive(:new).and_return(fake_octokit_client)
expect(fake_octokit_client).to receive_message_chain('user.login')
expect(fake_octokit_client).to receive(:pull_requests).with(repo_name, :state => 'open').and_return([pull_2266, pull_2248])
expect(fake_octokit_client).to receive(:search_issues).with("is:pr state:open user:alphagov").and_return(double(items: [pull_2266, pull_2248]))

allow(fake_octokit_client).to receive(:issue_comments).with(repo_name, 2266).and_return(comments_2266)
allow(fake_octokit_client).to receive(:issue_comments).with(repo_name, 2248).and_return(comments_2248)
Expand Down
8 changes: 3 additions & 5 deletions spec/seal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@
{
'lion' => {
'members' => [],
'repos' => ['leo'],
'use_labels' => nil,
'exclude_labels' => nil,
'exclude_titles' => nil,
},
'tigers' => {
'members' => [],
'repos' => ['stripes'],
'use_labels' => nil,
'exclude_labels' => nil,
'exclude_titles' => nil,
Expand Down Expand Up @@ -44,7 +42,7 @@
it 'fetches PRs for the tigers and only the tigers' do
expect(GithubFetcher)
.to receive(:new)
.with([], ['stripes'], nil, nil, nil)
.with([], nil, nil, nil)
.and_return(instance_double(GithubFetcher, list_pull_requests: []))

seal.bark
Expand All @@ -60,12 +58,12 @@
it 'fetches PRs for the lions and the tigers' do
expect(GithubFetcher)
.to receive(:new)
.with([], ['leo'], nil, nil, nil)
.with([], nil, nil, nil)
.and_return(instance_double(GithubFetcher, list_pull_requests: []))

expect(GithubFetcher)
.to receive(:new)
.with([], ['stripes'], nil, nil, nil)
.with([], nil, nil, nil)
.and_return(instance_double(GithubFetcher, list_pull_requests: []))

seal.bark
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#
require 'timecop'

ENV['SEAL_ORGANISATION'] ||= "alphagov"

RSpec.configure do |config|

config.expect_with :rspec do |expectations|
Expand Down

0 comments on commit 12f37c6

Please sign in to comment.