Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wip/cleanup for play #18

Merged
merged 10 commits into from
Jul 12, 2013
Merged

Wip/cleanup for play #18

merged 10 commits into from
Jul 12, 2013

Conversation

jsuereth
Copy link
Member

  • Add the formal bash/bat script generation to the JavaApp archetype
  • Cleanup the JavaApp archetype settings + documentation so it's one definition for the simplest projects
  • Add tests to ensure that vanilla dist and stage in the sbt prompt work
  • Add tests for the JavaApp archetype
  • Cleanup the script templating in the JavaApp archetype.

Review by @huntc with the following notes/caveats:

  1. make-bash-script and make-bat-script return Option[File] rather than File. This means we could default to None if we had to. Right now JavaApp archetype would be ok either way. I'm on the fence as to how we should actually do this. You'll have to use the java app archetype settings to get these defaults though, which I think is the right thing for play right now.
  2. The bat file remains untested until I pass this code off to the windows box. Wanted to get this in font of your eyes, as I'll be on windows the rest of the day.
  3. You may have to manually specify mainClass in play, unless it already does this.

Let me know what you think.

jsuereth added 5 commits July 10, 2013 12:35
* Add `makeBashScript` key to global Keys.
* Update JavaAppbashScript to use `makeBashScript` key.
* Move BASH script itself into resources as a template file.
* Update Java app archetype to be friendlier
* Add new test for non-prefixed `stage` and `dist` tasks
* Adding test to ensure autogenerated bash script works.
@jsuereth
Copy link
Member Author

Oh, also, @huntc -> I cut a milestone release of this code @ 0.6.0-milestone-for-ch-1. Ping me if I need to clarify any bits :)

declare -r java_cmd=$(get_java_cmd)
# TODO - Check whether this is ok in cygwin...
declare -r lib_dir="${app_home}/../lib"
declare -r app_classpath="$lib_dir/*"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be able to express the classpath as a parameter to the template. The order of things on the classpath is significant to Play as we try to be compatible with the order of things in the classpath during dev mode.

I've defined a taskkey named "playDistLibs" that yields a Seq(File, String). If the product of something like that can be fed to the template then that'd be great.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, actually enforcing is order may be something more than just play specific. Let me think about a good way to resolve the general case, and I'll add a quick hook for you now.

jsuereth added 2 commits July 11, 2013 10:11
* Use same classpath ordering as sbt in start scripts
* Allow user configuration of additional hooks for the script.

Review by @huntc to see if we're any closer to parity with Play.
* Consolidate names to use "script"
* Add better descriptions
* Update the docs a bit.
@jsuereth
Copy link
Member Author

Cool, I'm going to merge and test again on windows (just in case), then cut a 0.6.0. We can make updates to the java application during the 0.6.x series because it's still experimental. I know we'll need more hooks over time too.

jsuereth added a commit that referenced this pull request Jul 12, 2013
@jsuereth jsuereth merged commit cb78d4f into master Jul 12, 2013
@jsuereth jsuereth deleted the wip/cleanup-for-play branch July 12, 2013 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants