github / janky

Continuous integration server built on top of Jenkins and Hubot
MIT License
2.76k stars 237 forks source link

Add support for Slack #205

Closed mbartmann closed 9 years ago

mbartmann commented 10 years ago

Added a new chat service and fixed some minor leftovers from the transition to string room ids.

Build notifications in slack: screen shot 2014-05-11 at 15 19 27

mbartmann commented 10 years ago

Thanks for pointing this out @parkr.

As of the internal switch to string room ids, we can be sure that ids loaded from db are strings (https://github.com/github/janky/blob/master/lib/janky/branch.rb#L70), but campfire room ids are numbers, with zero as an illegal room id.

I think we have two options here: a) check every occasion of a room id parameter and cast to string, such as:

room_id = room_id.to_s
if room_id.nil? || room_id.empty? || room_id == "0"
  room_id = repository.room_id
end

b) cast an incoming room id to string at the API endpoint, so all room ids are strings internally (https://github.com/github/janky/blob/master/lib/janky/hubot.rb#L38)

mbartmann commented 10 years ago

@parkr I've changed the room id handling including its test.

I would love to see this addition merged into Janky to get us back to master. So if there is anything else I could do, your help would be much appreciated.

mattr- commented 9 years ago

Could you rebase this so it's up to date with the latest master?

mbartmann commented 9 years ago

@mattr- we are now up to date with the latest master.