Github user unsleepy22 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163318356
looks good to me now~
---
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 thi
Github user ashnazg commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163299110
Ahhh... Eclipse gave me the wrong import (org.apache.storm.guava.base)...
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163292661
@ashnazg adding a new dep in this case is not that big of a deal, because
it is only for a test. Just be sure to mark the dep as a test dependency in
maven if you do nee
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163267032
@ashnazg
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Joiner.html
---
If your project is set up for it, you can reply to this email and
Github user ashnazg commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163269805
Ok, let me doublecheck my build... the dep wasn't found, which is why I
thought "surely I don't need to add a new dep for this" :)
---
If your project is set up for it,
Github user unsleepy22 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163266835
@ashnazg use Joiner in guava (should be already in dependency) :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHu
Github user ashnazg commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163265892
Umm... is Joiner only available in Java8?
---
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
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-163045433
The change looks god +1. I agree with @unsleepy22 though and a joinPath
would make the code more readable, but it is optional in my mind.
---
If your project is set up
Github user unsleepy22 commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-162933358
I'd prefer to create a util method like `joinPath` which is like:
```
public static String joinPath(String... pathList) {
return Joiner.on(File.separator
Github user wuchong commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-162734475
+1 nice catch
---
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
e
Github user redsanket commented on the pull request:
https://github.com/apache/storm/pull/928#issuecomment-162696112
+1 NB
---
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
GitHub user ashnazg opened a pull request:
https://github.com/apache/storm/pull/928
Use File.separator for OS independence in Blobstore LocalizerTest
This clears up one incidental issue shown on STORM-1378, the one test
failure due solely to "/" vs "\" in the path comparison. This
12 matches
Mail list logo
