[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1691 @raghavgautam I am +1 overall. I believe it makes sense to have these integration tests run in the mvn integration-test phase, but that's the only detail I have to add. +1 --- If your project

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1691 @raghavgautam Nope. All is good. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 @ptgoetz Do we need to document any other place or just mentioning on the jira is sufficient ? --- If your project is set up for it, you can reply to this email and have your reply appear on Gi

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1691 Just to be clear, that wasn't a criticism. I just wanted to point out that it is important that we know the provenance and license of all code that enters our repository. --- If your project is set

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 I had mention this on the jira. A good part of the vagrant setup has been picked up from: https://github.com/ptgoetz/storm-vagrant https://github.com/harshach/storm-vagrant ht

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1691 This a fairly large commit that seemingly includes code from other projects. That's fine as long as you can document what code was copied, and what the license for that code was. --- If your projec

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 Thanks @harshach @HeartSaVioR for reviewing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this fe

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1691 I squashed commits before merging (b779ca4e270f6f2a86d4d30336004e4a642ec690) but forgot to write 'Closes #1691' to commit log. @raghavgautam Could you close this? Thanks! --- If your p

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1691 @raghavgautam Thanks. It looks great to me. +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this f

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 @HeartSaVioR I have fixed the rat issue and I have put fast fail. Yes, we will be contributing more tests to apache storm going forward. --- If your project is set up for it, you can re

[GitHub] storm issue #1691: STORM-2090: Add integration test for storm windowing

Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1691 @raghavgautam 1. Travis CI fails with RAT issue. Could you run your branch via `mvn clean install -Prat`? 2. Could we apply fail-fast? I think integration tests don't need to be run when