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
GORA-509: Upgrade solr library to 6.5.1 #104
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @madhawa-gunasekara this is looking good.
Can you also please just remove the existing gora-solr module and rename gora-solr-5 to gora-solr?
@@ -0,0 +1,16 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add license header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
@@ -0,0 +1,14 @@ | |||
# Licensed to the Apache Software Foundation (ASF) under one or more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file required if there is nothing in it?
@@ -0,0 +1,17 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add license header please
@@ -0,0 +1,14 @@ | |||
# Licensed to the Apache Software Foundation (ASF) under one or more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this if there is nothing in it?
@@ -0,0 +1,9 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
License header please.
/** | ||
* Created by madhawa on 5/28/17. | ||
*/ | ||
public class GoraSolr6TestDriver extends GoraSolrTestDriver { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you make sure your code is formatted using 2 space indents?
+ "not only removes the data but also the data structure.", 0, page.getOutlinks().size()); | ||
assertEquals(0, page.getParsedContent().size()); | ||
if (page.getContent() != null) { | ||
LOG.info("url:" + page.getUrl().toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use parameterized logging as follows
LOG.info("url: {}", page.getUrl().toString());
pom.xml
Outdated
@@ -739,12 +739,12 @@ | |||
<!-- Solr Dependencies --> | |||
<lucene-solr.version>4.10.3</lucene-solr.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the old lucene-solr.version
pom.xml
Outdated
<lucene-solr-5.version>5.5.1</lucene-solr-5.version> | ||
<solr-solrj-5.version>5.5.1</solr-solrj-5.version> | ||
<jetty.version>8.1.8.v20121106</jetty.version> | ||
<lucene-solr-5.version>6.5.1</lucene-solr-5.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this to lucene-solr.version
pom.xml
Outdated
<jetty.version>8.1.8.v20121106</jetty.version> | ||
<lucene-solr-5.version>6.5.1</lucene-solr-5.version> | ||
<solr-solrj-5.version>6.5.1</solr-solrj-5.version> | ||
<jetty.version>9.3.14.v20161028</jetty.version> | ||
<tika.version>1.7</tika.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Tika is a transient dependency made available through Solr (which I think it most definitely is), please make sure to make the update here as well.
30480f0
to
d07787f
Compare
@lewismc Thanks, |
@madhawa-gunasekara great work :) Thanks for the PR. |
@lewismc Are you done reviewing the PR? Shall I proceed merging this to master? |
+1 |
@madhawa-gunasekara I am getting following error when I build the PR changes locally can you please check? [INFO] ------------------------------------------------------------------------ |
@djkevincr I have fixed it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged the PR. Thank you very much @madhawa-gunasekara :)
@djkevincr Thanks a lot 👍 |
No description provided.