Add Janky::GitHubStatus #104

Merged
merged 10 commits into from Sep 12, 2012

5 participants

@rsanheim
GitHub member

See https://github.com/blog/1227-status-api for more info on the Status API.

@rsanheim
GitHub member

/cc @sr

@eric

❤️

@ejholmes

Yay!

@sr
sr commented Sep 5, 2012

:metal:

I've updated this a bit to automatically setup the notifier when JANKY_GITHUB_STATUS_TOKEN is set and support Enterprise (obey JANKY_GITHUB_API_URL basically).

What's a curl command that people can run to get a repo:status token? I'd love to throw that in the README then we can release this and push it up to rubygems.org.

Updated github/ci to run this as well.

@rsanheim
GitHub member

@sr Not sure of the curl command to get a token, it would be something from here. Haven't dug in to actually test it yet, though.

@eric

Something like this?

$ curl -u login:password -d '{ 'scopes': [ 'repo:status' ], 'note': 'janky' }' https://api.github.com/authorizations
@maletor

@eric nailed it.

@eric

I must be doing something else wrong, though, because none of the notifications seem to fire.

@eric eric commented on an outdated diff Sep 5, 2012
lib/janky/notifier/github_status.rb
+ repo = build.repo_nwo
+ path = "repos/#{repo}/statuses/#{build.sha1}"
+ status = build.green? ? "success" : "failure"
+
+ desc = case status
+ when "success" then "Build ##{build.number} succeeded in #{build.duration}s"
+ when "failure" then "Build ##{build.number} failed in #{build.duration}s"
+ end
+
+ post(path, status, build.web_url, desc)
+ end
+
+ # Internal: POST the new status to the API
+ def post(path, status, url, desc)
+ http = Net::HTTP.new(@api_url.host, @api_url.port)
+ post = Net::HTTP::Post.new("#{@api_url.path}/#{path}")
@eric
eric added a line comment Sep 5, 2012

This extra / causes the URL to be //repos which returns a 404.

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

To prevent Problems parsing JSON error.

curl -u username:password -d '{ "scopes": [ "repo:status" ], "note": "janky" }' https://api.github.com/authorizations
@rsanheim
GitHub member

Merging this. Ping @sr or anyone else who knows the release process -- do we normally just bump the gem in gemspec and push it out?

@rsanheim rsanheim commented on the diff Sep 12, 2012
lib/janky.rb
@@ -198,7 +199,14 @@ def self.setup(settings)
end
ChatService.setup(chat_name, chat_settings, chat_room)
- Notifier.setup(Notifier::ChatService)
+ if token = settings["JANKY_GITHUB_STATUS_TOKEN"]
+ Notifier.setup([
+ Notifier::GithubStatus.new(token, api_url),
+ Notifier::ChatService
@rsanheim
GitHub member
rsanheim added a line comment Sep 12, 2012

er was this a mistake @sr?

@rsanheim
GitHub member
rsanheim added a line comment Sep 12, 2012

Ah I guess I understand it. Man that setup method is a beast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rsanheim rsanheim merged commit ca23b54 into master Sep 12, 2012

1 check passed

Details default Build #99457 succeeded in 11s
@maletor

@rsanheim this branch is broken.

@rsanheim
GitHub member

care to elaborate?

@rsanheim
GitHub member

I removed the slash in a commit in this PR.

@maletor

@rsanheim I see now. Yours is much better, but still, there is a pending migration for users upgrading.

@rsanheim
GitHub member

See 8703397

@rsanheim
GitHub member

@maletor Is there a pull or issue for the pending migration? Not familiar with that change off hand.

@rsanheim
GitHub member

@maletor @ejholmes Please open separate issues (or preferably pull requests!) for both those items with links to the correct things.

Sorry, I didn't work on those changes and it is a bit too late now for me to be trying to wrangle those items :).

@ejholmes ejholmes referenced this pull request Dec 12, 2012
Merged

Add post install message. #106

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