Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for custom HipChat installs #2

Merged
merged 2 commits into from
Aug 12, 2015
Merged

Add option for custom HipChat installs #2

merged 2 commits into from
Aug 12, 2015

Conversation

ciwchris
Copy link
Contributor

@ciwchris ciwchris commented Aug 7, 2015

A optional HipChat BaseUrl has been added to allow publishing notifications to a user specified URL, instead of only to a hosted HipChat install.

A optional HipChat BaseUrl has been added to allow publishing notifications to a user specified URL, instead of only to a hosted HipChat install.
@ciwchris
Copy link
Contributor Author

ciwchris commented Aug 7, 2015

I don't know how to install a locally generated Seq App, so I wasn't able to completely test my change.

@@ -52,7 +60,7 @@ public class HipChatReactor : Reactor, ISubscribeTo<LogEventData>
{
using (var client = new HttpClient())
{
client.BaseAddress = new Uri("https://api.hipchat.com/v2/");
client.BaseAddress = new Uri(HipChatBaseUrl ?? DefaultHipChatBaseUrl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contrib!

I'm unsure about Seq's behaviour, but it would feel more safe using string.IsNullOrWhitespace to check for empty value.

Check the following comment for how to run locally: #1 (comment)

It is unknown exactly how Seq sets optional properties. Therefore, to be safe, the optional HipChat base URL is now checking for both null and whitespace, instead of using the coalescing operator.
@ciwchris
Copy link
Contributor Author

Updated. Thanks for the tip on using a local nuget feed, I don't know how I missed it!

kristofferlindvall added a commit that referenced this pull request Aug 12, 2015
Add option for custom HipChat installs
@kristofferlindvall kristofferlindvall merged commit 7a53dda into stayhard:master Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants