Release/2.1 #13

Merged
merged 18 commits into from Dec 11, 2011

3 participants

@sfgeorge
Collaborator

G'day! This is a follow-up to my previous ssl pull request. I would pull (push?) this myself, but I think it deserves a good code review or two. It includes a number of changes:

Features

  • Added support for submitting errors and deployments over https.
  • All optional Toadhopper host settings are now exposed for Capistrano deployments, including :airbrake_secure, :airbrake_notify_host, :airbrake_error_url, and :airbrake_deploy_url.

Bugs

  • API Errors returned from a failed deploy notification were not being captured.
  • Capistrano: the [deprecated] api key setting was not being read.

Miscellaneous

  • Changed default notification domain to airbrake.io.
  • toadhopper is now tested in a continuous integration environment, thanks to travis-ci.org. The build is currently Build Status for rubies 1.8.7, 1.9.2, 1.9.3, ree (1.8.7-2011.03), jruby (1.6.3), and rubinius (2.0 in 1.8.7 mode). woot!
@sfgeorge
Collaborator

@toolmantim, at your leisure... can you set up the primary travis CI account for the main repo?

I set up a temporary CI account under http://travis-ci.org/sfgeorge/toadhopper to make sure things are passing.

No rush... that not a blocker for merging/releasing... the release/1.0 branch is a candidate for a 'fast-forward' merge to master. So, since the build is currently Build Status on release/1.0, it should pass when merged to master as well.

@morgoth
Collaborator

Looks good to me.
Great job 👍

@sfgeorge
Collaborator

Thanks v much! :)

@toolmantim
Owner

Nice one! 👍

@toolmantim toolmantim merged commit 2066c5f into toolmantim:master Dec 11, 2011
@toolmantim
Owner

Added to Travis too, just waiting for it to pop off the queue. Feel free to release to Rubygems!

@sfgeorge
Collaborator

Thanks.... but...

Curses! Curses! Many, many curses! Another issue with SSL! ...

I just tested this on one of my boxes running CentOS 5.7 and SSL is not working when connecting to https://airbrake.io. It does work if I specify a :notify_host of https://airbrakeapp.com. This is because my system's default (stale) SSL CA file cannot verify airbrake.io.

We have at least 3 choices here:
1. Disable SSL peer verification (yikes!)
2. Set the default notify domain to airbrakeapp.com (would help for most users, but not all)
3. Use the bundled SSL CA file in the toadhopper gem by default

I vote for option 3. It seems this is a necessity for a lot of folks who use the Net::HTTPS module, including Thoughtbot's airbrake project.

Thoughtbot recently (just 4 hours ago!) resolved this exact same issue in v3.0.8 by always using a pre-bundled SSL CA file instead of using the default (possibly stale) file on the local operating system. We would likely benefit by using the same approach. However, due to the recency of their release, let's let it stew for a bit and verify that it works well for them.

In short, let's sit tight for a little while before releasing v2.1. Sorry. :/

@toolmantim
@sfgeorge sfgeorge referenced this pull request Apr 23, 2012
Closed

Push to latest? #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment