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

Proxy Support #7

Closed
wants to merge 3 commits into from
Closed

Proxy Support #7

wants to merge 3 commits into from

Conversation

jplock
Copy link
Contributor

@jplock jplock commented Dec 15, 2014

Fixes #6

@hbakkum
Copy link
Owner

hbakkum commented Dec 16, 2014

Hi,

Thanks for submitting this request. Proxy changes look good, I'll test this out today and merge in the changes. Can I ask why the @OVERRIDES gives you errors in eclipse - is this because source level is set to 1.5 or something else?

Thanks,
Hayden

@jplock
Copy link
Contributor Author

jplock commented Dec 16, 2014

I'm not sure. I'm using JDK8 at this point with the latest eclipse and it would not compile with those @Override's in there

@hbakkum
Copy link
Owner

hbakkum commented Dec 17, 2014

Ok, I've tried the latest eclipse with Java 7 and I do get the same errors. It seems by default eclipse sets the compiler compliance level to 1.5 even though I'm using Java 7. Changing the compliance level to 1.6+ addresses the issue so I'm going to leave those @OVERRIDES in there.

So I've just cherry-picked those other commits. I made a few changes on top of those:

  • Proxy configuration did not seem to take effect when I was testing. This was because the proxy settings were not actually being passed to the HipChatRoomNotifierFactory - I've fixed that
  • I collapsed the proxy host/port set up into a single method.

I've now released that as version 1.5.0 of the plugin - should sync out to maven central in the next hour or so.

Thanks again for this, please let me know if you have any more feedback in the plugin or if there are any other features you'd like to see.

Cheers,
Hayden

@hbakkum hbakkum closed this Dec 17, 2014
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.

Add proxy support
2 participants