Added the ability to specify the zulip url and port.

zulip / zulip-redmine-plugin

Redmine plugin for Zulip notifications on issue tracker changes

Apache License 2.0

12 stars 17 forks source link

Closed ogg1e closed 8 years ago

timabbott commented 8 years ago

Thanks for contributing this @ogg1e !

I posted one comment, but otherwise this lgtm, assuming you've tested this and have signed the Dropbox CLA: https://opensource.dropbox.com/cla/

ogg1e commented 8 years ago

I committed the change to switch the API based on the URL. I tested it as best as I could on a local install, but not on zulip.com.

I also have signed the CLA

timabbott commented 8 years ago

Thanks @ogg1e ! A few small tweaks:

Can you squash the two commits together?
Can you fix the commit author name/email to not be "root"? You can do this using e.g. env GIT_AUTHOR_NAME="Gary Ogden" GIT_AUTHOR_EMAIL=<email> git commit --amend --reset-author
Can you rename zulip_api_basename to zulip_api_endpoint if it's going to include the /messages part? Alternatively you could leave it the funciton as zulip_api_basename and concatenate strings in the req = Net::HTTP::Post.new(zulip_api_basename()) line.

ogg1e commented 8 years ago

I had some issues with the squash, but I think I've got it right now.

timabbott commented 8 years ago

Thanks @ogg1e ! This looks great except that this seems to have lost the commit author fix somewhere in the rebase; can you fix that?

ogg1e commented 8 years ago

There, I think it's all correct now. Sorry about that.

timabbott commented 8 years ago

Merged, thanks for fixing this @ogg1e !!