search

fabric8io / openshift-elasticsearch-plugin

Apache License 2.0
27 stars 21 forks source link

Bumping up to work with ES 2.3.3, SG-2 and SG-SSL #29

Closed ewolinetz closed 8 years ago

ewolinetz commented 8 years ago

@jcantrill PTAL

fusesource-ci commented 8 years ago

License check failed: run mvn -N license:format to update all licenses, commit, squash & force push please.

jcantrill commented 8 years ago

Mostly nits and questions for things to think about around multi-threading scenarios. Also, add some tests please.

fusesource-ci commented 8 years ago

License check failed: run mvn -N license:format to update all licenses, commit, squash & force push please.

ewolinetz commented 8 years ago

@jimmidyson the license-check is expecting the year to be "2015" and is failing with me having "2016" should that be updated?

jimmidyson commented 8 years ago

Up to you. We tend to leave it the same across the project as AFAIK the copyright runs for more than long enough to cover any reasonable time period.

ewolinetz commented 8 years ago

@jcantrill thoughts on above?

jcantrill commented 8 years ago

They were new files added this year so the license check should account for that...but it's really not that important to me

On Thursday, August 4, 2016, Eric Wolinetz notifications@github.com wrote:

@jcantrill https://github.com/jcantrill thoughts on above?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fabric8io/openshift-elasticsearch-plugin/pull/29#issuecomment-237608279, or mute the thread https://github.com/notifications/unsubscribe-auth/AEVnOLiBeoSb3gnBO0bcWypakAcv251uks5qchQCgaJpZM4JYf7s .

Jeff Cantrill Senior Software Engineer, Red Hat Engineering OpenShift Integration Services Red Hat, Inc. Office: 703-748-4420 | 866-546-8970 ext. 8162420 jcantril@redhat.com http://www.redhat.com

ewolinetz commented 8 years ago

@jimmidyson lets leave it as is for now then I suppose. @jcantrill Addressed nits -- for locking I updated to match what we were doing with our previous version.

fusesource-ci commented 8 years ago

License check failed: run mvn -N license:format to update all licenses, commit, squash & force push please.

ewolinetz commented 8 years ago

added tests, probably need to update readme too... but that may not be necessary in this PR

jcantrill commented 8 years ago

LGTM...best I can tell

ewolinetz commented 8 years ago

Okay, we may need to refactor since that other pr was merged... If not and you're good with this we can merge it.

On Aug 6, 2016 3:28 PM, "Jeff Cantrill" notifications@github.com wrote:

LGTM...best I can tell

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/fabric8io/openshift-elasticsearch-plugin/pull/29#issuecomment-238047175, or mute the thread https://github.com/notifications/unsubscribe-auth/ABXIEeayAlyYgsI438et2TvAu-Q_3Hbwks5qdO5agaJpZM4JYf7s .

ewolinetz commented 8 years ago

@jcantrill https://github.com/fabric8io/openshift-elasticsearch-plugin/issues/30 is to not lose the README changes for this PR

ewolinetz commented 8 years ago

@jcantrill hold off on merging this actually... i just came across a NPE in kibana_seed when deleting a namespace

ewolinetz commented 8 years ago

@jcantrill PTAL

ewolinetz commented 8 years ago

@jcantrill found the source of the issue for the NPE. PTAL

jcantrill commented 8 years ago

LGTM after squash

ewolinetz commented 8 years ago

squashed

ewolinetz commented 8 years ago

[merge]