You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by krichter722 <gi...@git.apache.org> on 2015/09/15 21:58:58 UTC

[GitHub] storm pull request: added missing installation instructions for in...

GitHub user krichter722 opened a pull request:

    https://github.com/apache/storm/pull/737

    added missing installation instructions for installation of rvm and n…

    …vm of DEVELOPER.md (without running \`mvn clean install\`) isn't possible on an average system (like Ubuntu 15.04) and subheadings to increase readability

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

    $ git pull https://github.com/krichter722/storm readme

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

    https://github.com/apache/storm/pull/737.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 #737
    
----
commit 1fb2a20021cea28466a140542331fb2ee1aa1177
Author: Karl-Philipp Richter <kr...@aol.de>
Date:   2015-09-15T19:53:19Z

    added missing installation instructions for installation of rvm and nvm of DEVELOPER.md (without running \`mvn clean install\`) isn't possible on an average system (like Ubuntu 15.04) and subheadings to increase readability

----


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140561321
  
    > Python doesn't have package manager so if you don't have python, you need to install manually.
    
    I agree with you. We don't need that sentence, just providing download page link should be enough.
    
    > With python, ruby and nodejs installed I'm getting the (extremely unclear) error Clojure failed - I can provide more details if you want (I found talios/clojure-maven-plugin#80 which speculates that Clojure failed indicates missing binaries) -, so that imo rvm and nvm are a requirement 
    
    RVM and NVM is just a package manager for giving users convenience, not requirement. 
    (I use RVM but don't use NVM on my Mac, and I don't have any issues.)
    Users can still install specific version of ruby / node manually, by homebrew in OSX, etc.
    
    Btw, you can get ```Clojure failed``` whenever any clojure tests error occur, so it doesn't tell you have an issue related to multilang.
    You should open target/test-reports to see what actual tests are failed.
    
    As you can see (from Travis CI), storm-core suffers from random failures, especially slow machines.
    When you provide actual errors from test-reports I'll 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 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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140564025
  
    Great, +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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140910301
  
    @krichter722 Thanks for the work, I merged into master.


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140578684
  
    +1. Thanks for contributing!


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140570580
  
    @krichter722 Great idea. Could you add it too?


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140563170
  
    > Btw, you can get Clojure failed whenever any clojure tests error occur, so it doesn't tell you have an issue related to multilang.
    You should open target/test-reports to see what actual tests are failed.
    
    > As you can see (from Travis CI), storm-core suffers from random failures, especially slow machines.
    When you provide actual errors from test-reports I'll take a look.
    
    Offer appreciated. I'll open a separate issue if necessary. After installating `rvm` and `nvm` everything works fine - except for the random test failures, of course.
    
    What do you think about the new commit?


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140565438
  
    > Btw, you can get Clojure failed whenever any clojure tests error occur, so it doesn't tell you have an issue related to multilang.
    You should open target/test-reports to see what actual tests are failed.
    
    I've no insight in clojure, the `maven-clojure-plugin` and its usage in `storm`, but in case that's a generic way to deal with such failures (with really incredibly unhelpful feedback) it should be added to the `Testing` section as well, right?


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140576591
  
    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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#issuecomment-140553259
  
    Thanks for your feedback. I'm just getting started with (building) `storm`.
    
    > Python doesn't have package manager so if you don't have python, you need to install manually.
    
    I don't understand completely. Do you mean that `python` doesn't have a built-in package manager which allows bootstrapping `python.` Is that really necessary to mention? And is it important where people get their `python` from (source installation, package managers)? -> I'd leave users with the information that they need `python` & co and add links for famous OSs if people start to ask too many questions on the mailing list.
    
    > continue explaining RVM and NVM with clarifying it's for convenience, not requirement.
    
    With `python`, `ruby` and `nodejs` installed I'm getting the (extremely unclear) error `Clojure failed` - I can provide more details if you want (I found https://github.com/talios/clojure-maven-plugin/issues/80 which speculates that `Clojure failed` indicates missing binaries) -, so that imo `rvm` and `nvm` are a requirement - if not other build instructions are necessary because `mvn clean install` doesn't work out-of-the-box, like one might think after reading `DEVELOPER.md`. What is your insight on this?


---
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] storm pull request: added missing installation instructions for in...

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

    https://github.com/apache/storm/pull/737#discussion_r39566345
  
    --- Diff: DEVELOPER.md ---
    @@ -222,6 +222,19 @@ To pull in a merge request you should generally follow the command line instruct
     
     # Build the code and run the tests
     
    +## Prerequisites
    +You need the `ruby` package manager `rvm` which can be installed using `curl -L https://get.rvm.io | bash -s stable --autolibs=enabled && source ~/.profile` (see the [rvm installation instructions](https://github.com/rvm/rvm) for details) and the `nodejs` package manager `nvm` which can be installed using `wget -qO- https://raw.githubusercontent.com/creationix/nvm/v0.26.1/install.sh | bash && source ~/.bashrc` (see the [nvm installation instructions](https://github.com/creationix/nvm) for details).
    --- End diff --
    
    @krichter722 
    It's better to clarify that Storm actually requires Python / Ruby / Node, not RVM, and NVM.
    So we would better to explain below first, 
    ```
    You need the Python, Ruby, Node to build the codes.
    Python doesn't have package manager so if you don't have python, you need to install manually. (see the [python download page](https://www.python.org/downloads/) for details)
    ```
    and continue explaining RVM and NVM with clarifying it's for convenience, not requirement. Users can still install Ruby and Node manually.


---
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.
---