Conversation
jorgeazevedo
added
the
maintenance
jorgeazevedo
force-pushed
the
ja-use-standard-sbt
branch
4 times, most recently
from
6328e32 to
00de658
Compare
jorgeazevedo
force-pushed
the
ja-use-standard-sbt
branch
from
00de658 to
df338c2
Compare
adamnfish-gu
left a comment
adamnfish-gu
left a comment
There was a problem hiding this comment.
A couple of minor questions, but great to tidy this up.
| @@ -0,0 +1,3 @@ | |||
| -Xms1024M | |||
There was a problem hiding this comment.
Did you test it without these, I doubt they're required! Might be simpler to omit them if they aren't essential, but no harm.
There was a problem hiding this comment.
Yeah I don't think these are actually needed, but I thought it'd be simpler to just leave them in rather than to understand why they were put there in the first place and test if the application works as expected etc
build.sbt
Outdated
|
|
||
| Test / javaOptions += "-Dconfig.file=conf/application.test.conf" | ||
|
|
||
| Runtime / javaOptions += s"-Dconfig.file=${System.getProperty("user.home")}/.gu/amiable.local.conf" |
There was a problem hiding this comment.
I think Runtime affects Test as well so I'm not 100% sure how these two settings interact? That said, the config isn't currently loaded in the tests so this isn't a huge deal either way as far as I can tell.
There was a problem hiding this comment.
This is a great catch! I didn't realise these interact with each other. Test ends up getting a duplicate config.file option
$ sbt 'show Runtime / javaOptions'
[...]
[info] * -Dconfig.file=~/.gu/amiable.local.conf
$ sbt 'show Test / javaOptions'
[...]
[info] * -Dconfig.file=~/.gu/amiable.local.conf
[info] * -Dconfig.file=conf/application.test.confWhat do you think of using PlayKeys.devSettings instead?
So something like
PlayKeys.devSettings += "config.file" -> s"${System.getProperty("user.home")}/.gu/amiable.local.conf"Remove the custom ./sbt script that bundled sbt-launch.jar and replace it with the standard sbt command. JVM memory options are now configured in .jvmopts, and the local dev config file is set via build.sbt (run / javaOptions). This aligns with modern sbt best practices where sbt is installed on the machine and fetches the right version from project/build.properties.
jorgeazevedo
force-pushed
the
ja-use-standard-sbt
branch
from
8498974 to
c434256
Compare
2 participants
What does this change?
This retires the old
sbtwrapper script and switch to using standardsbt.This required finding a way to apply the same JVM options (which I'm not really sure are necessary tbh but I found it easier to just maintain it as is) and specifying the local config file in a different way.
There a few different ways to specify this config file, but I ended up opting for an identical implementation to frontend https://github.com/guardian/frontend/blob/436fee5dd6787fcdff9f68bb2699d85d7c89d049/build.sbt#L167
Note: the original debug call specified
-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=1056", which is no longer necessary with modern sbt as the -jvm-debug flag already sets these exact properties.How has this change been tested?
The application boots properly when running locally