Fix missing require, working hipchat support. #26

Merged
merged 11 commits into from Jan 27, 2012

Conversation

Projects
None yet
3 participants
@jsmestad
Contributor

jsmestad commented Jan 7, 2012

Think this was missed in the latest merge and I have verified working hipchat support with these changes.

@schisamo

This comment has been minimized.

Show comment
Hide comment
@schisamo

schisamo Jan 10, 2012

Contributor

@jsmestad good catch on this stuff. @sr we will def want to merge these few fixes/enhancements

Contributor

schisamo commented Jan 10, 2012

@jsmestad good catch on this stuff. @sr we will def want to merge these few fixes/enhancements

@@ -5,13 +5,14 @@ def self.completed(build)
status = build.green? ? "was successful" : "failed"
color = build.green? ? "green" : "red"
- message = "Build #%s (%s) of %s/%s %s (%ss) %s" % [
+ message = "Build #%s (%s) of %s/%s %s (%ss) <a href='%s'>%s</a>" % [

This comment has been minimized.

@sr

sr Jan 27, 2012

Contributor

Reverting this, won't work in Campfire.

@sr

sr Jan 27, 2012

Contributor

Reverting this, won't work in Campfire.

@@ -156,7 +155,7 @@ def self.setup(settings)
chat_name = settings["JANKY_CHAT"] || "campfire"
chat_settings = {}
settings.each do |key, value|
- if key =~ /^JANKY_CHAT_#{chat_name}_/
+ if key =~ /^JANKY_CHAT_#{chat_name}_/i

This comment has been minimized.

@sr

sr Jan 27, 2012

Contributor

Reverting this as well, we always want uppercases here.

@sr

sr Jan 27, 2012

Contributor

Reverting this as well, we always want uppercases here.

This comment has been minimized.

@jsmestad

jsmestad Jan 27, 2012

Contributor

The readme was outdated saying "hipchat" was valid for that var.

Sent from my iPhone

On Jan 26, 2012, at 6:12 PM, Simon Rozet
reply@reply.github.com
wrote:

@@ -156,7 +155,7 @@ def self.setup(settings)
chat_name = settings["JANKY_CHAT"] || "campfire"
chat_settings = {}
settings.each do |key, value|

  •  if key =~ /^JANKY_CHAT_#{chat_name}_/
    
  •  if key =~ /^JANKY_CHAT_#{chat_name}_/i
    

Reverting this as well, we always want uppercases here.


Reply to this email directly or view it on GitHub:
https://github.com/github/janky/pull/26/files#r390602

@jsmestad

jsmestad Jan 27, 2012

Contributor

The readme was outdated saying "hipchat" was valid for that var.

Sent from my iPhone

On Jan 26, 2012, at 6:12 PM, Simon Rozet
reply@reply.github.com
wrote:

@@ -156,7 +155,7 @@ def self.setup(settings)
chat_name = settings["JANKY_CHAT"] || "campfire"
chat_settings = {}
settings.each do |key, value|

  •  if key =~ /^JANKY_CHAT_#{chat_name}_/
    
  •  if key =~ /^JANKY_CHAT_#{chat_name}_/i
    

Reverting this as well, we always want uppercases here.


Reply to this email directly or view it on GitHub:
https://github.com/github/janky/pull/26/files#r390602

@sr

This comment has been minimized.

Show comment
Hide comment
@sr

sr Jan 27, 2012

Contributor

Awesome, thanks. Sorry for the delay.

Contributor

sr commented Jan 27, 2012

Awesome, thanks. Sorry for the delay.

sr added a commit that referenced this pull request Jan 27, 2012

Merge pull request #26 from jsmestad/patch-1
Fix missing require, working hipchat support.

@sr sr merged commit 4738159 into github:master Jan 27, 2012

@@ -35,7 +35,7 @@ class Hubot < Sinatra::Base
repo = find_repo(repo_name)
branch = repo.branch_for(branch_name)
build = branch.current_build
- room_id = params["room_id"] && Integer(params["room_id"])
+ room_id = params["room_id"] && Integer(params["room_id"]) rescue nil

This comment has been minimized.

@sr

sr Jan 27, 2012

Contributor

What was the issue here? Does Hubot/HipChat not send a room_id or something?

@sr

sr Jan 27, 2012

Contributor

What was the issue here? Does Hubot/HipChat not send a room_id or something?

This comment has been minimized.

@jsmestad

jsmestad Jan 27, 2012

Contributor

Hipchat supports private messages which does not have a room id. This fixes that issue.

@jsmestad

jsmestad Jan 27, 2012

Contributor

Hipchat supports private messages which does not have a room id. This fixes that issue.

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