You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@joshua.apache.org by logogin <gi...@git.apache.org> on 2016/06/16 08:31:32 UTC

[GitHub] incubator-joshua pull request #24: Maven multi-module project layout proposa...

GitHub user logogin opened a pull request:

    https://github.com/apache/incubator-joshua/pull/24

    Maven multi-module project layout proposal

    Hey Joshua Team
    This is a proposal for the maven multi-module project layout. Please, do not merge - lets discuss it first.
    This change contains the following proposed layout:
    - joshua-parent
      - joshua-core 
      - joshua-servlet
    
    In the future we may add `joshua-cli` and etc. The idea is to strip out core functionality from the side features like HTTP server.
    `joshua-servlet` is actually an adapter to the Servlet 3.1 API. It will allow users to supply any relevant server implementation (Tomcat or Jetty). 
    `joshua-servlet/pom.xml` already contains predefined minimal Jetty configuration to run on `8080`.
    To build whole project (also runs test in sub-modules) just run:
    ```
    mvn install
    ```
    from the parent. To run jetty execute
    ```
    mvn jetty:run
    ```
    from `joshua-servlet` module.
    Please share your thoughts and comments about it. I will continue to update this pull request according to your feedback if needed.

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

    $ git pull https://github.com/logogin/incubator-joshua maven-multi-module

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

    https://github.com/apache/incubator-joshua/pull/24.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 #24
    
----
commit 523ce0677f5902d128c33ea7b2a2807ce39d8303
Author: Pavel Danchenko <da...@amazon.com>
Date:   2016-06-07T12:22:59Z

    maven multi-module project layout; added joshua-servlet module to HTTP server functionality

----


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Hi @mjpost . I wanted general opinion on the proposal but looks mostly people support it. Some things to take into account if that will go into master:
    * joshua-servet contains only basic functionality and does not represent a drop-in replacement for the [ServerThread](https://github.com/apache/incubator-joshua/blob/55e88d1fce0e2654d090c14d999e43acd6ac8820/src/main/java/org/apache/joshua/server/ServerThread.java) as of now. The idea was to get something working, collect opinions and extend if necessary.
    * The configuration of the Decider supported only via `decoderArgsLine` init parameter. There are many ways to extend that (i.e. via system property). But I am just unsure which way is preferable.
    * Translation with request body payload now requires POST method instead of GET. Although that is more natural for the RESTful application, it might cause some incompatibilities with existing tools.
    * The artifacts for the corresponding modules are `joshua-core-<version>.jar` and `joshua-servlet-<version>.jar` respectively. This might require updating some bash scripts or tools with hard-coded artifact names. If that is the case, please list those here and I will look for the solutions.
    * Since folder layout also changed, above might be applicable here as well.
    * Probably the most important - after that change, there will be no more updates to `joshua-<veriosn>.jar` in the maven repository. We will have to follow this naming convention and support new artifact naming. So that is pretty crucial point and if we have better naming or modules hierarchy, we need to settle it before merging with master.
    
    But if you are ok with all those point we can definitely proceed and merge. Thank to the git time machine we can travel to the past :). 


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Boom goes the dynamite
    
    On Aug 17, 2016 3:33 AM, "Matt Post" <no...@github.com> wrote:
    
    > Awesome, that merged really cleanly. There is now a pushed 7 branch that
    > we can begin work on for release 7. Travis-CI
    > <https://travis-ci.org/apache/incubator-joshua/builds/152931806> is
    > testing this as I write.
    >
    > Once we release 6.1, I'll merge 7 into master.
    >
    > \u2014
    > You are receiving this because you commented.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/incubator-joshua/pull/24#issuecomment-240374918>,
    > or mute the thread
    > <https://github.com/notifications/unsubscribe-auth/ABHJl5m5Z5FWe2PClnnsEKZs-vGOwTuKks5qguOXgaJpZM4I3JQs>
    > .
    >



---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    No big deal, but if you update this pull request to be against the "7" branch instead of master, it will be marked as resolved.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Re-based on top of master. 
    I also split the change into two separate commits to simplify the review: the 1st one is just moving files, no changes; the 2nd contains actual modifications and new 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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    This looks cool to me. Is this a recent pattern or something? I just noticed a similar approach on the [deeplearning4j](https://github.com/deeplearning4j/deeplearning4j) repo.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    @mjpost so is the proposal to include this in 6.1? Or 7 only?


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    This is dynamite. I'll pull later and check it out.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Yeah, that was one of our main concerns, too. Which is why we want to get this into master as soon as possible. There are some nasty merges ahead of us, but the unit tests should help pull us through :)


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Commenting here so this discussion gets tied to issue [JOSHUA-295](https://issues.apache.org/jira/browse/JOSHUA-295?jql=project%20%3D%20JOSHUA%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20key%20DESC%2C%20priority%20DESC). A re-implementation of this will serve as the base for Joshua 7, currently scheduled for spring release.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    We wanted to include this in 6.1, but felt that a responsible manager would say that it is non-critical and that we should instead base our future release on it.
    
    We rebuilt this yesterday on top of master. Within the next few days, the new version should get pushed up. I will pull that into a "7" branch, which we'll start work on. Once 6.1 is officially released, we'll merge "7" back into master.
    
    Sound okay?


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by chrismattmann <gi...@git.apache.org>.
Github user chrismattmann commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    LOVE 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] incubator-joshua issue #24: Maven multi-module project layout proposal

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Sounds like an excellent roadmap. I would hate to see things get too out of sync so I will be conscious of that. Excellent work folks.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Just to clarify, so the idea is to move components into separate modules that could be compile and included by parent module?


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    updated pull request to reflect recent master commits; fixed bash script tests with copying fat-jar into parent module


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Awesome, that merged really cleanly. There is now a pushed 7 branch that we can begin work on for release 7. [Travis-CI](https://travis-ci.org/apache/incubator-joshua/builds/152931806) is testing this as I write.
    
    Once we release 6.1, I'll merge 7 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] incubator-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    When I change base to "7" it says no more new commits. I thing it can be closed as resolved.


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by KellenSunderland <gi...@git.apache.org>.
Github user KellenSunderland commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    This PR is related to our discussion in this Joshua ticket.  We felt it might be good to have some code people can play with to see how maven submodules could be used. 
    
    https://issues.apache.org/jira/browse/JOSHUA-274  
    



---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by mjpost <gi...@git.apache.org>.
Github user mjpost commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    Thanks, @logogin. I have only glanced at this, but it seems like a good idea. I hope to be able to take a look tonight, and expect all will be well. Do you want more discussion, or should I merge tonight if it seems okay to me?


---
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-joshua pull request #24: Maven multi-module project layout proposa...

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

    https://github.com/apache/incubator-joshua/pull/24


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    re-beased again; migrated test to TetsNG


---
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-joshua issue #24: Maven multi-module project layout proposal

Posted by logogin <gi...@git.apache.org>.
Github user logogin commented on the issue:

    https://github.com/apache/incubator-joshua/pull/24
  
    @mjpost The pattern is just a standard one. I was not aware of naming in deeplearning4j.
    @hsaputra Yes, that is correct. The code base tends to become monolith peace of software which is hard to refactor. Also keeping core functionality clean will allow better integration of the Decoder as embedded system.


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