everypolitician / close_old_pull_requests

Auto-close everypolitician-data pull requests that have been superseded (where it is safe to do so)

EveryPolitician

Data about every national legislature in the world, freely available for you to use

• everypolitician.org — data | about
• Report an issue

Repo summary

These are some of the key repos in the EveryPolitician family. There are others.

• everypolitician (this repo): contains no code, but is where issues/tickets for the whole project live

• everypolitician-data: where the data is stored -- but if you want to download it, get it from:

  • human? go via the EveryPolitician website
  • program? use the RawGit CDN, via links in countries.json, which we explain here

• viewer-static: the live website http://everypolitician.org (gh-pages)

• viewer-sinatra: Sinatra app for generating a dynamic version EveryPolitician website

• webhook-manager: sends out EveryPolitician WebHooks: register your URL here!

• everypolitician-docs: documentation at http://docs.everypolitician.org/ (gh-pages)

• rebuilder rebuilds data from source

• libraries for easily manipulating EveryPolitician data (useful for all devs, but we use the Ruby ones ourselves, of course!):

  • Ruby: everypolitician-ruby and everypolitician-popolo.
  • Python: everypolitician and everypolitician-popolo

• handy gems we use when getting the data: wikidata-fetcher, wikisnakker, twitter_username_extractor, facebook_username_extractor, twitter_list, scraped_page_archive

• gender-balance: repo for the Gender Balance website that crowdsources gender data for EveryPolitician

• data_pr_change_summarizer: code used by the bot to review a data PR and leave a helpful summary as a comment

The repos for many of our scrapers are kept separately in github.com/everypolitician-scrapers.

Technical blog

The EveryPolitician bot's own page is a good jumping-off point to lots of semi-technical explanations of what's going on (it has its own blog on Medium). For example:

• how the website is built (spoiler: viewer-sinatra → viewer-static)
• how webhooks are used (you can easily register your app!)
• how the scrapers run (many live on morph.io)

The bot is on twitter as @everypolitician

Contributing

If you have data for us, or know where to get it, please read our page about how to contribute.

Team

EveryPolitician is a mySociety project.

close_old_pull_requests's Issues

Add a rake task

Much like we did in everypolitician/everypolitician-pull_request#6 we should add a rake task for this project so that using it in a consuming application can be a two liner, one line to require the rake task, and one line to install the rake tasks.

More useful output

At the moment the output of this command is:

Pull request 16560 is outdated. (Newest pull request is 16748)
Pull request 16592 is outdated. (Newest pull request is 16738)
Pull request 16678 is outdated. (Newest pull request is 16721)

It would be more useful it it actually said what action had been taken for each pull request.

Make the bot username explicit

At the moment we're assuming that the GitHub access token belongs to the bot, then we're using github.user.login to get the name of the bot's GitHub account. I think it would be less confusing to just hardcode the bot's GitHub login (but perhaps make it overridable). Without this we end up with confusing messages like this one.

Pull requests superseded by themselves

Problem

On a couple of pull requests, e.g. everypolitician/everypolitician-data#13834 (comment), the bot has posted a comment saying that the pull request has been superseded by itself.

Proposed solution

I'm not sure why this is happening, but a simple fix it to add a check that the comment being posted isn't about the PR that it's being posted on.

Finder should return the full pull request

Problem

I want to add the url for the pull request to the message that I print out for #7, but currently the Finder class only returns a PullRequest struct with the number of the pull request (and a superseded_by number if there's a newer pull request).

Proposed solution

The Finder#outdated method should return the pull request objects that Octokit returns so that the url and other properties can be accessed for logging purposes. This might be easiest done as a hash so we can do something like the following:

finder.outdated do |outdated_pull_request, new_pull_request|
  # …
end

Acceptance criteria

At this point in Cleaner#clean_old_pull_requests I should be able to access the Octokit pull request instance for the outdated and the new pull requests.

Delete branch after closing

It's currently quite a cumbersome task to clean up the gazillion old branches in https://github.com/everypolitician/everypolitician-data, a large number of which get left behind by this job.

This means that the repo tends to balloon until it's unusable (See everypolitician/everypolitician#620)

@henare has suggested that this process could not only close the branch, but also delete it too, which sounds like a Good Idea™ to me!

Don't leave multiple comments on a single pull request

If the bot has already commented on an open pull request then it shouldn't comment again, otherwise we end up with a situation where a pull request gets a comment on it each time it's run.

Only run against pull requests from the bot

Problem

We're currently running this library against all pull requests, but actually we should only be running them against pull requests from the bot. Otherwise we end up in a situation like everypolitician/everypolitician-data#25100 and everypolitician/everypolitician-data#25102 where two human generated pull requests have the same title and then you get a confusing message from the bot.

Proposed solution

Only run against pull requests from the bot.

Handle commits with no author information

Problem

We're seeing errors like this in the Travis build:

rake aborted!
NoMethodError: undefined method `[]' for nil:NilClass
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:45:in `block in author_logins'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:45:in `map'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:45:in `author_logins'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:53:in `empty?'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:72:in `block in clean_old_pull_requests'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:67:in `each'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:67:in `clean_old_pull_requests'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/bundler/gems/close_old_pull_requests-ba2dc4ebe77e/lib/close_old_pull_requests.rb:12:in `clean'
/home/travis/build/everypolitician/everypolitician-data/Rakefile.rb:45:in `block in <top (required)>'
/home/travis/build/everypolitician/everypolitician-data/vendor/bundle/ruby/2.3.0/gems/rake-11.2.2/exe/rake:27:in `<top (required)>'

Looks like this was because it had encountered a commit which had no author key attached to it because the email used for the commit wasn't associated with a GitHub account.

# Proposed solution

We should check for a commit with a null author and handle that case correctly.

Notes for reproducing

To reproduce this error you'll need to create a commit using an email address that isn't associated with a GitHub account.