sbt / sbt-atmos

sbt plugin for running Typesafe Console in development
98 stars 11 forks source link

Added support for running consle-traced Play applications with snapshot #9

Closed corruptmemory closed 11 years ago

corruptmemory commented 11 years ago

releases of atmos.

Used by: https://github.com/typesafehub/atmos/pull/365

pvlugter commented 11 years ago

I have doubts about these changes.

I don't see the need for this when atmosVersion is already a setting that can be changed. For example:

sbt 'set AtmosKeys.atmosVersion in Atmos := "1.3.0-SNAPSHOT"' atmos:run

This would be my preference, and allows it to work for any branch and version of atmos. And it's more flexible. For example, the snapshot setup won't work when atmos master moves to 1.4.0-SNAPSHOT and sbt-atmos is using the stable version 1.3.x.

If atmosVersion was to be set via system property then it only needs to be changed in one place:

https://github.com/sbt/sbt-atmos/blob/master/akka/src/main/scala/com/typesafe/sbt/SbtAtmos.scala#L81

but settings can also be modified from the command-line, without special support, as above.

corruptmemory commented 11 years ago

Mostly fair-enough. Setting via system property is certainly the most important feature that needs support. But there is a second important feature:

https://github.com/corruptmemory/sbt-atmos/blob/support-snapshots/akka/src/main/scala/com/typesafe/sbt/atmos/AtmosRunner.scala#L51

This is nice because it avoids the entire ProGuard step.

Happy to make the changes to AtmosVersion.

pvlugter commented 11 years ago

What's the need for system property instead of using set command?

Skipping the proguard step can be done by only using sbt-atmos to run the tracing (with traceOnly setting) and then running query and console from the atmos build, as normal. This allows console to run in play development mode with reloading, while testing against the play demos.

corruptmemory commented 11 years ago

OK, updated. More to your taste? Other suggestions welcome.

pvlugter commented 11 years ago

Needs a rebase -- one of the files was renamed yesterday.

And will need the scala dependency hack... but's lets wait with that until we need it for atmos development work.

pvlugter commented 11 years ago

Rebased and merged manually in e6689fd4bf6b70e6233f0e3b742cfe370b5afd51.