tbroyer / gwt-maven-archetypes

Apache License 2.0
152 stars 39 forks source link

Simplify profile configuration #13

Closed jgonian closed 12 years ago

jgonian commented 12 years ago

To simplify the profile configuration of a generated project, I merged the production configuration settings to the main body of the POM and removed 'prod'. Now, there are only two profiles left (located in the client module) and they have to be activated explicitly.

The noticeable differences are the initial mvn install instead of mvn package and that you don't have to specify a profile when starting jetty.

Regarding issue #12, I haven't set dev as the default profile since it is now trivial for a user to set it (just a matter of changing a single line in the client's POM file). Under this setup, I tend to agree that the default configuration should be the "web mode" and let users decide if they want to change this default configuration or not.

tbroyer commented 12 years ago

I tend to avoid using install because then you don't know which exact version of other submodules you're using, and that's why the dev profile excludes the dependencies so you don't have to install them first.

I don't consider myself an advanced Maven user though, so it might just be me (and everyone else use install a lot, when I rather use package a lot and verify from time to time, along with -pl, -am and-amd).

I haven't found any evidence that install is the way Maven is supposed to be used, neither that my approach is better.

If there's enough traction for this pull-request (I know there are a few watchers/lurkers), or if you can point me to some evidence that this is the way Maven is supposed to be used, then I'll put my tastes apart and merge this.


That put apart, now that we'd no longer have an "active-by-default" profile, are the system properties still needed/best-practice for profile activation or couldnt we just use -Pdraft and -Pdev instead? (changing the README accordingly)

jgonian commented 12 years ago

I must admit I don't consider the mvn install-way the best practice, because it takes more time to build everything when you could just build the dependencies you need (as you do). However, that implies a deeper understanding of the project dependencies which is - especially for big multi-module projects - impractical (speaking from personal experience).

Generally speaking though, I have the feeling the install goal is one of the most common goals in the Maven world. When you first checkout a project you normally run mvn install in order to verify that it builds successfully and in order to place the dependencies in your local repo so that you can later build only the modules you are working with (without using -pl, -am etc).

Of course, there is always the chance to use outdated dependencies from your local repo, but you can always run mvn clean install once more and get again up-to-date. The fact that you need to remember only a single command makes mvn clean install more appealing to most Maven developers.

Nevertheless, I'd really like to hear more voices on this issues since I don't think there are any best practices or references documented regarding this specific topic.

About the profile activation, I agree the properties are not needed anymore. I could amend this pull request if you want.

tbroyer commented 12 years ago

Let's ask a Maven expert. Cc @olamy

olamy commented 12 years ago

I don't know if jetty plugin handle modules from reactor without installation (I think no). So in such case install is mandatory. Note the tomcat maven plugin handle correctly modules from reactors so you don't need to install first all artifacts. :P

tbroyer commented 12 years ago

@olamy tomcat unfortunately doesn't help here: mvn tomcat7:run -pl :basic-server -am will run the basic-client module, because it has a war packaging too. Using the SNAPSHOT version and using the skip configuration in the client module allows running the server module, but it fails when trying to extract the client war:

[ERROR] fail to extract war file […]/client/target/classes, reason:The source must not be a directory.

I also tried using Sonatype's Webby for M2Eclipse (won't honor the loginService from the plugin configuration in the POM, I suppose we could use a jetty-web.xml in WEB-INF, but that hardly be more than a workaround, which is what we're precisely trying to avoid; or point Webby to an installed and configured Jetty or Tomcat container), and M2Eclipse's "Resolve Workspace artifacts", with a jetty:run without specific configuration besides the loginService (I have a 404 after authenticating, didn't investigate why I doesn't find the index.jsp, which is in the src/main/webapp)

So the question remains: should we simplify the POMs at the expense of having to install the dependencies, or keep things as they are (dependencies on other modules are not used in "dev" mode) at the expense of a more complicated POM? A compromise might be to use the tomcat maven plugin but (at least pending an update of the plugin) putting the dependency on the client module (war overlay) within a 'prod' profile as we have today. The only advantage would be to remove the baseResource and extraClasspath configurations (which you currently have to update as you add dependencies on other modules), as the tomcat maven plugin seems to do it automatically ("handle correctly modules from reactors").

What would be the best practice?

For now, I'm tempted to leave the project as it is now and wait for an update of the plugins.

jgonian commented 12 years ago

So the question remains: should we simplify the POMs at the expense of having to install the dependencies, or keep things as they are (dependencies on other modules are not used in "dev" mode) at the expense of a more complicated POM?

I would opt for the simpler POM configuration. If we consider that it's mandatory to install the dependencies once (only the first time that you build the project) I don't find it so troublesome.

On the other hand, the current configuration might be more sophisticated but it works, is flexible and it has clear boundaries.

tbroyer commented 12 years ago

OK, I'm convinced we should get this in.

Would you mind amending the commit to fix/enhance the README?

jgonian commented 12 years ago

Done.

tbroyer commented 12 years ago

I've fixed the formatting in the README and omitted 33f3072

tbroyer commented 12 years ago

FYI, I just found a workaround for the required mvn install: https://jira.codehaus.org/browse/JETTY-1517?focusedCommentId=306630&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-306630

A bit hackish, but far less than the initial solution based on profiles. It would still require some manual configuration though, as Maven 3's reactor-resolution only gives you the compiled classes for a JAR, not the sources (needed for gwt:run) neither the web resources (needed for jetty:run); and it would only temporarily work for jetty:run (because we use an overlay) thanks to a bug in Maven; and that bug is already fixed in trunk so it really is a temporary situation (this is the same bug that made tomcat7:run from above fail to extract war file).