You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by milantracy <gi...@git.apache.org> on 2018/08/15 20:38:22 UTC
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
GitHub user milantracy opened a pull request:
https://github.com/apache/storm/pull/2803
STORM-2578: Apply new code style to storm-elasticsearch
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/milantracy/storm storm2578
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2803.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2803
----
commit 174bc7cb9632336dd0e0947ba1e81916b1967ae4
Author: Jing Chen <mi...@...>
Date: 2018-08-15T20:34:57Z
STORM-2578: Apply new code style to storm-elasticsearch
----
---
[GitHub] storm issue #2803: STORM-2578: Apply new code style to storm-elasticsearch
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2803
@milantracy Thanks for the contribution! Could you please reduce the number of max violation count so that we can see how many spots your patch address, and also we never break it again?
---
[GitHub] storm issue #2803: STORM-2578: Apply new code style to storm-elasticsearch
Posted by milantracy <gi...@git.apache.org>.
Github user milantracy commented on the issue:
https://github.com/apache/storm/pull/2803
@HeartSaVioR could you review the changes? Thanks.
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2803#discussion_r211055345
--- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/doc/Index.java ---
@@ -28,6 +29,13 @@ public Index() {
}
+ /**
+ * Create a Index with the specified index, type and id.
+ *
+ * @param index index name
+ * @param type document type to be stored
+ * @param id unique document id in Elastisearch
--- End diff --
nit: Elastisearch -> Elasticsearch
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2803#discussion_r211055054
--- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/EsLookupResultOutput.java ---
@@ -25,17 +26,22 @@
import org.elasticsearch.client.Response;
/**
- * @since 0.11
* The adapter to convert the results fetched from Elasticsearch to values.
+ *
+ * @since 0.11
*/
public interface EsLookupResultOutput extends Serializable {
/**
+ * Covert Elasticsearch response to a collection of {@link Values}.
--- End diff --
nit: Covert -> Convert
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by milantracy <gi...@git.apache.org>.
Github user milantracy commented on a diff in the pull request:
https://github.com/apache/storm/pull/2803#discussion_r210499451
--- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/bolt/EsLookupBolt.java ---
@@ -76,12 +78,12 @@ public void process(Tuple tuple) {
}
private Collection<Values> lookupValuesInEs(Tuple tuple) throws IOException {
- String index = tupleMapper.getIndex(tuple);
- String type = tupleMapper.getType(tuple);
- String id = tupleMapper.getId(tuple);
- Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
+ String index = tupleMapper.getIndex(tuple);
+ String type = tupleMapper.getType(tuple);
+ String id = tupleMapper.getId(tuple);
+ Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
--- End diff --
hi Danny, we might be not allowed to indent with tab because of FileTabCharacter in checkstyle. Please correct me if I misunderstood here.
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2803#discussion_r210478306
--- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/bolt/EsLookupBolt.java ---
@@ -76,12 +78,12 @@ public void process(Tuple tuple) {
}
private Collection<Values> lookupValuesInEs(Tuple tuple) throws IOException {
- String index = tupleMapper.getIndex(tuple);
- String type = tupleMapper.getType(tuple);
- String id = tupleMapper.getId(tuple);
- Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
+ String index = tupleMapper.getIndex(tuple);
+ String type = tupleMapper.getType(tuple);
+ String id = tupleMapper.getId(tuple);
+ Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
--- End diff --
should indent with Tab
---
[GitHub] storm issue #2803: STORM-2578: Apply new code style to storm-elasticsearch
Posted by milantracy <gi...@git.apache.org>.
Github user milantracy commented on the issue:
https://github.com/apache/storm/pull/2803
Hi @HeartSaVioR , reduced the value to 0.
---
[GitHub] storm issue #2803: STORM-2578: Apply new code style to storm-elasticsearch
Posted by milantracy <gi...@git.apache.org>.
Github user milantracy commented on the issue:
https://github.com/apache/storm/pull/2803
thanks @HeartSaVioR, change the fix accordingly.
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2803#discussion_r211055229
--- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/bolt/EsLookupBolt.java ---
@@ -76,12 +78,12 @@ public void process(Tuple tuple) {
}
private Collection<Values> lookupValuesInEs(Tuple tuple) throws IOException {
- String index = tupleMapper.getIndex(tuple);
- String type = tupleMapper.getType(tuple);
- String id = tupleMapper.getId(tuple);
- Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
+ String index = tupleMapper.getIndex(tuple);
+ String type = tupleMapper.getType(tuple);
+ String id = tupleMapper.getId(tuple);
+ Map<String, String> params = tupleMapper.getParams(tuple, new HashMap<>());
--- End diff --
It was broken indentation (mixed tab and space) and the change fixes the indentation.
---
[GitHub] storm pull request #2803: STORM-2578: Apply new code style to storm-elastics...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2803
---