You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by xpe <gi...@git.apache.org> on 2014/05/31 20:06:49 UTC

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

GitHub user xpe opened a pull request:

    https://github.com/apache/incubator-storm/pull/130

    move towards idiomatic Clojure style

    First, some background, when I first dig into a code base, I list to get a rough idea what's inside by reading it over. I like to fix some obvious style problems while I'm going along.
    
    Summary:
    * When using defn, put function arguments on a separate line.
    * Remove dangling ),],}
    * Try to keep lines at 80 characters or less
    
    Going forward, I’d like to see
    * more docstrings
    * most lines under 80 characters
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/bluemont/incubator-storm idiomatic-clojure-01

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-storm/pull/130.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 #130
    
----
commit 2278fc9623d5b71bca9ece3857e645e85ab1562f
Author: David James <da...@bluemontlabs.com>
Date:   2014-05-31T17:55:54Z

    move towards idiomatic Clojure style
    
    Summary:
    * When using defn, put function arguments on a separate line.
    * Remove dangling ),],}
    * Try to keep lines at 80 characters or less
    
    Going forward, I’d like to see
    * more docstrings
    * most lines under 80 characters

----


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45163796
  
    @revans2 Hello. The quality of the Storm code varies. It could stand some improvement. Just like any project that is useful enough to live on, it spans versions. It also spans the improvement in coding style of its contributors.
    
    I'm with you; I don't want an arbitrary debate. I'd suggest "defining" Storm's desired style, roughly, as what 80% of the Clojure community is currently doing. The rest could be left open. Over time, it probably will distill itself with little work on our part.
    
    What is the 80% of less-than-controversial advice? I suggest we point people to these useful references (to answer your documentation question):
    * http://dev.clojure.org/display/community/Library+Coding+Standards
    * https://github.com/bbatsov/clojure-style-guide
    
    > Is there an automated tool that you used that can help with this formatting as well?
    
    Many people use Paredit or their editor. If I see a lint tool, I will let you know. I see that https://github.com/jonase/eastwood is one kind of linter; it might be worth a look. I'll keep my eyes open.
    
    @revans2 I'll submit a pull request. Can you merge quickly? The longer we wait, the more merge conflicts we'll get.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45624196
  
    @xpe Thanks, I am now +1 on this change, it would be great to get another commiter to also give a +1 so that we can get this merged in and stop with the upmerging.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45627789
  
    +1 
    Glad to see this didn't spark a religious debate. :)


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45925762
  
    > If there are no objections in the next few hours I will try to merge this in, and fix the spelling myself.
    
    +1 go for it.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/incubator-storm/pull/130#discussion_r13599793
  
    --- Diff: conf/defaults.yaml ---
    @@ -94,6 +94,9 @@ supervisor.enable: true
     worker.childopts: "-Xmx768m"
     worker.heartbeat.frequency.secs: 1
     
    +# control how many worker receiver threads we need per worker 
    --- End diff --
    
    We have documentation in Config.java.  Do we want to duplicate it here?


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45632088
  
    Having code style standards is a good idea, and I'm for it.
    
    * No objection to the style chosen.
    
    * With this change, there is a git-blame "barrier."  Say I am searching for the revision a particular change was made: It seems to me I will hit this commit for a large amount of the lines in the clojure source instead of locating the commit where the last non-formatting change was made.  Anyone know a way around this or a better way of locating a commit for a particular line of code?
    
    * To practically help with compliance to this standard, is there an automated tool we could use to validate changes or is it up to the reviewers to manually do this?
    
    * One tiny spelling comment



---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45923584
  
    There are two +1's but @d2r had the one spelling fix that needed to go in.  I think the question of tools is something that will be ongoing.  If there are no objections in the next few hours I will try to merge this in, and fix the spelling myself.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-storm/pull/130


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45210755
  
    Some counter-points:
    * To grep for a function, look for `defn function-name`. You don't need the arguments.
    
    It is better to have a docstring for each function. This means the args need to be on a different line.
    
    Putting everything on the first line may "fool" people into thinking a docstring is not needed. This is a bad habit to fall into.
    
    
    
    For multi-arity functions, you won't get the arguments on the 
    



---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on a diff in the pull request:

    https://github.com/apache/incubator-storm/pull/130#discussion_r13262494
  
    --- Diff: storm-core/src/clj/backtype/storm/util.clj ---
    @@ -36,10 +37,10 @@
       (:require [clojure.java.io :as io])
       (:use [clojure walk])
       (:use [backtype.storm log])
    -  )
    +  (:refer-clojure :exclude [some?]))
    --- End diff --
    
    Sorry, I meant to exclude this line.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by miguno <gi...@git.apache.org>.
Github user miguno commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45181599
  
    > I like the idea of cleaning up the style of the clojure.
    
    I second Bobby's opinion, and yours, @xpe.
    
    I'd agree with @knusbaum though to keep argument lists on the same line -- if only because then you can `grep` the code base on the command line if you're quickly looking for a function definition.
    
    That being said I'm not religious about which style as long as it's consistent, and if following the style is supported by IDE (you already mentioned paredit) in some way.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/incubator-storm/pull/130#discussion_r13599791
  
    --- Diff: DEVELOPER.md ---
    @@ -92,6 +92,14 @@ and
     [trivial issues](https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&jqlQuery=project+%3D+STORM+AND+resolution+%3D+Unresolved+AND+priority+%3D+Trivial+ORDER+BY+key+DESC&mode=hide))
     because they require learning about only an isolated portion of the codebase and are a relatively small amount of work.
     
    +Please use idiomatic Clojure style, as explained in [this Clojure style guide][clj-SG]. Another useful reference is
    +the [Clojure Library Coding Standards][clj-LCS]. Perhaps the most important is consistenly writing a clear docstring
    --- End diff --
    
    consisten[t]ly (sorry)


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by miguno <gi...@git.apache.org>.
Github user miguno commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45222031
  
    @xpe All good points.  I stand corrected. :-)
    
    +1 to Bobby's suggestion of updating DEVELOPER.md as well.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45945887
  
    Thank you all very much; sorry I didn't get the last parts dialed in.
    
    > On Jun 12, 2014, at 10:57 AM, "P. Taylor Goetz" <no...@github.com> wrote:
    > 
    > If there are no objections in the next few hours I will try to merge this in, and fix the spelling myself.
    > 
    > +1 go for it.
    > 
    > —
    > Reply to this email directly or view it on GitHub.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45218451
  
    @xpe I totally agree that upmerging is a pain, so I think we can probably get this in soon.  Then the pain is mine to upmerge all of my outstanding patches :).
    
    I would like to see a section added to DEVELOPER.md describing the coding style, pointing to the other web pages you listed, and also the suggestions for IDEs that can help with the formatting.
    
    I would also like to see the title of this pull request edited to have STORM-338 in it.  The project has some tools that tie the two together using the title.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45167927
  
    The only part I have a little concern about is the arguments on a separate line of function defns. 
    I really prefer the argument lists to be on the same line as the definition, since I find it more readable. 
    
    The style guides you proposed actually suggest that you include the arguments on the same line unless there is a docstring, and I would really prefer to follow that, rather than applying the rule indiscriminately. 
    
    That's my only request. I won't make a debate out of it.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: Move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45164474
  
    See https://issues.apache.org/jira/browse/STORM-338


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45571871
  
    @revans2 Done.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/incubator-storm/pull/130#discussion_r13599959
  
    --- Diff: conf/defaults.yaml ---
    @@ -94,6 +94,9 @@ supervisor.enable: true
     worker.childopts: "-Xmx768m"
     worker.heartbeat.frequency.secs: 1
     
    +# control how many worker receiver threads we need per worker 
    --- End diff --
    
    Hmmm, this looks like a change from the upmerge.  Disregard.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45940897
  
    Glad to see that we could get this in.  Hopefully it will not cause too many headaches for others up-merging.  I think I am the one that will have the biggest headache with the security branch :).


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45924850
  
    > If there are no objections in the next few hours I will try to merge this in, and fix the spelling myself.
    
    +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 feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-44755388
  
    The reason I forked was to prevent these warnings:
    
    ```clj
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.util, being replaced by: #'backtype.storm.util/some?
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.clojure, being replaced by: #'backtype.storm.util/some?
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.thrift, being replaced by: #'backtype.storm.util/some?
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.config, being replaced by: #'backtype.storm.util/some?
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.scheduler.DefaultScheduler, being replaced by: #'backtype.storm.util/some?
    WARNING: some? already refers to: #'clojure.core/some? in namespace: backtype.storm.scheduler.EvenScheduler, being replaced by: #'backtype.storm.util/some?
    ```


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-44755252
  
    ```
    Results :
    
    Tests run: 35, Failures: 0, Errors: 0, Skipped: 0
    
    [INFO] ------------------------------------------------------------------------
    [INFO] Reactor Summary:
    [INFO] 
    [INFO] Storm ............................................. SUCCESS [0.614s]
    [INFO] maven-shade-clojure-transformer ................... SUCCESS [1.114s]
    [INFO] Storm Core ........................................ SUCCESS [2:07.361s]
    [INFO] storm-starter ..................................... SUCCESS [3.184s]
    [INFO] storm-kafka ....................................... SUCCESS [23.383s]
    [INFO] ------------------------------------------------------------------------
    [INFO] BUILD SUCCESS
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time: 2:36.234s
    [INFO] Finished at: Sat May 31 14:01:20 EDT 2014
    [INFO] Final Memory: 20M/446M
    [INFO] ------------------------------------------------------------------------
    ```


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45087919
  
    Oh I also forgot to add that all pull requests need a JIRA associated with them.  Please file a JIRA in http://issues.apache.org/jira if there is not one already there then add the jira number, aka STORM-...., to the title of this pull request.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: move towards idiomatic Clojure style

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45087241
  
    I like the idea of cleaning up the style of the clojure. For me I learned clojure working on storm, and a lot of the code I write has fairly arbitrary formatting.  I have looked through some, but not all of the code and what I have seen seems OK to me.  I just really have a few concerns.
    
    First is this somehow going to turn into a "religious" debate.  I looked around and could not find much of anything about formatting of clojure code, except a stackoverflow saying format it like lisp.  I couldn't find any submission guidelines about it on clojure.org either.  Is this just an unwritten standard, or is there going to be a debate about this?
    
    Second is there documentation anywhere that we can use to describe the formatting?  After doing something like this it seems logical to me that we would want to try and have new pull requests also follow the same guidelines, but we need something to point submitters to that they can use for this purpose. Is there an automated tool that you used that can help with this formatting as well?
    
    My final concern is actually rather minor, but pulling this in is going to cause some pair for anyone else who has a pull request outstanding, simply because it touches so many different files.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-388 Move towards idiomatic Clo...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45356881
  
    Can you change the name to STORM-338, not STORM-388?


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45508586
  
    Thanks @xpe,  If you can update DEVELOPER.md and then upmerge, there appear to be a few minor conflicts, that would be great.


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by miguno <gi...@git.apache.org>.
Github user miguno commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45629138
  
    I havent' had the time to walk through the code, but the least I can do is to confirm that all tests pass on my machine when running a top-level `mvn clean install`.
    
    As shown in the pull request there are 4 conflicting files if you try to merge @xpe's changes into `master`;  I told git to overwrite those 4 files with their the remote versions (i.e. @xpe's changes).


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-storm pull request: STORM-338 Move towards idiomatic Clo...

Posted by xpe <gi...@git.apache.org>.
Github user xpe commented on the pull request:

    https://github.com/apache/incubator-storm/pull/130#issuecomment-45386497
  
    @revans2 Done. Sorry about that!


---
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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---