Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
I am merging I am still happy to make changes on follow on JIRA.
---
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
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1744
I only really have nitpicks, so I'm good with merging too. Really nice job.
---
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 projec
Github user erikdw commented on the issue:
https://github.com/apache/storm/pull/1744
@revans2 : Since I didn't have any substantive blocking commits I'm ok with
merging it. And it's been long enough that I think you're probably ok to merge
away. Any hidden problems will hopefully be
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
@erikdw I addressed your nits. I appreciate the nits because we don't have
a coding standard yet.
If we are just down to nits I really would like to get this in.
---
If your project is set
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
I just rebased and I think I have also addressed all of the review comments
so far.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1744
nimbus.clj, I made it to mkAssignments. Will continue from there later:
- nit: makeBlobCachMap -> makeBlobCacheMap
- change: `numDesiredWorkers < numAssignedWorkers` to `numDesiredWorkers
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
@ptgoetz I totally agree on refactoring it. I did a little as I ported it
over, but nothing like it needs. I also have done some manual testing.
---
If your project is set up for it, you can reply
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1744
Nice (and a lot of ;) ) work!
Nimbus.java is a little unwieldy, but I understand why. Maybe it can be a
target for some refactoring later on.
+1 (Note that I haven't manually tested
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
The test failures look unrelated. It would really be nice if someone could
fix the TumblingWindowTest and SlidingWindowTest. They fail way too often.
---
If your project is set up for it, you can
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
I just rebased on top of the latest master (with a java worker translation)
---
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 pro
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
Just fixed the merge conflict, and reran the tests
---
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 user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
@dossett if you want to build the branch and do some manual testing with
your own topologies especially looking at the UI that would be helpful. I did
it, but only with word count, and I looked at t
Github user dossett commented on the issue:
https://github.com/apache/storm/pull/1744
I don't know enough about nimbus or clojure to vote on this PR, but this is
an exciting change!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1744
All the code is ready if people want to take a look.
---
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
14 matches
Mail list logo
