[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread hmcl
Github user hmcl commented on the issue:

https://github.com/apache/storm/pull/2549
  
+1

Thanks @HeartSaVioR for working on this right away.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
@hmcl Addressed.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
@hmcl Got it. You're saying marking the origin commit offset. Makes sense. 
I'll do it.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread hmcl
Github user hmcl commented on the issue:

https://github.com/apache/storm/pull/2549
  
+1

@HeartSaVioR I meant to say that you should add also to the commit log the 
SHA of the 1.x-branch that you checked out and of which copied the entire 
storm-kafka-client directory and the examples. Otherwise, how do we know when 
in the 1.x-branch history this back-porting was done?


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
@hmcl Just applied your review comment.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
@hmcl 
Yes that's only `InterfaceStability` which was needed in storm-core for 
1.1.x-branch. I was a bit confused between 1.1.x-branch and 1.0.x-branch, which 
1.0.x-branch requires more changes on storm-core (I stopped working on 
1.0.x-branch since @erikdw was expressed his willing to work).

That's a good idea to list the files needed to modify in storm-core. I'll 
do it. Thanks!


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread hmcl
Github user hmcl commented on the issue:

https://github.com/apache/storm/pull/2549
  
@HeartSaVioR I agree that your approach. Can please add to the commit 
message the SHA from the 1.x-branch that was the base for this overwrite, such 
that one can trace it down in case it is necessary. Thanks.

I am not sure I am following when you mention compilation issues. I ran the 
following git command, and there seems to be no code changes.

```
git diff upstream/1.x-branch external/storm-kafka-client/
diff --git a/external/storm-kafka-client/pom.xml 
b/external/storm-kafka-client/pom.xml
index 51bb79771..0ba7d4d00 100644
--- a/external/storm-kafka-client/pom.xml
+++ b/external/storm-kafka-client/pom.xml
@@ -22,7 +22,7 @@
 
 storm
 org.apache.storm
-1.2.0-SNAPSHOT
+1.1.2-SNAPSHOT
 ../../pom.xml
 
``` 


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread srdo
Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2549
  
+1. Hope we can avoid something like this in the future. Thanks for the 
quick PR Jungtaek.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread ptgoetz
Github user ptgoetz commented on the issue:

https://github.com/apache/storm/pull/2549
  
+1


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
So the commit solely represents divergence between 1.x and 1.1.x in point 
of storm-kafka-client's view, and there's no basis to separate commits for this.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
Such iteration would be much harder for 1.0.x, since there're more changes 
on 'storm-core' applied in 1.1.x, hence storm-core is (surely) more diverged 
for storm-kafka-client.


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2549
  
@erikdw 
"fix compilation issues due to storm-core" <== maybe my bad to explain 
here, I don't fix something specific for compilation, but I was done 
iterations: build -> found error -> found what I've missed from storm-core -> 
apply -> build again. So the change should be actually along with 
storm-kafka-client.

There're lots of of commits for storm-kafka-client, and I can't track and 
apply one to one. If I should take that approach, then I can't make it such 
easier and faster.

archiving module was really simple: 
1. git checkout 1.x-branch
1. mvn clean
1. cd external
1. tar cvfz storm-kafka-client.tar.gz storm-kafka-client
1. mv storm-kafka-client.tar.gz ~
1. git checkout 1.1.x-branch
1. git checkout -b 
1. cd external
1. rm -rf storm-kafka-client
1. tar xvfz storm-kafka-client.tar.gz
1. rm -rf storm-kafka-client.tar.gz

and start iteration...


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread erikdw
Github user erikdw commented on the issue:

https://github.com/apache/storm/pull/2549
  
Also can you please tell me how you "archived" the couple of items that you 
did?


---


[GitHub] storm issue #2549: STORM-2936 Overwrite latest storm-kafka-client 1.x-branch...

2018-02-06 Thread erikdw
Github user erikdw commented on the issue:

https://github.com/apache/storm/pull/2549
  
I know we've talked with @hmcl about the "to squash or not to squash?" 
question -- IMNSHO this is a perfect example of where squashing is a bad idea.  
i.e., I have *no* idea what "fix compilation issues due to storm-core" actually 
entailed by just looking at this giant commit.


---