A bit of TLC for both the template and output README files #1
Conversation
…s the generated output. snuck in a couple typo fixes too
README.md
Outdated
| sbt --supershell=false new sbchapin/quickstart-scala-sbt-native-image.g8 | ||
| ``` | ||
|
|
||
| Running either one of these commands **will create a new folder in your active directory.** |
There was a problem hiding this comment.
I hate how this convolutes the start of your README, which used to be so clean and nice. The output from:
sbt --supershell=false new sbchapin/quickstart-scala-sbt-native-image.g8looks really good still, so another option might be to just remove all this bullshit I added and cherry pick just the new command.
There was a problem hiding this comment.
I used to think more documentation, more better. Now I think documentation density. Tomorrow I'll think I should have thought less.
Today, my suggestion is to replace the english with comments:
# sbt < 1.3.0
sbt new sbchapin/quickstart-scala-sbt-native-image.g8
# sbt >= 1.3.0 (https://github.com/sbt/sbt/issues/5063)
sbt --supershell=false new sbchapin/quickstart-scala-sbt-native-image.g8
README.md
Outdated
| sbt --supershell=false new sbchapin/quickstart-scala-sbt-native-image.g8 | ||
| ``` | ||
|
|
||
| Running either one of these commands **will create a new folder in your active directory.** |
There was a problem hiding this comment.
I used to think more documentation, more better. Now I think documentation density. Tomorrow I'll think I should have thought less.
Today, my suggestion is to replace the english with comments:
# sbt < 1.3.0
sbt new sbchapin/quickstart-scala-sbt-native-image.g8
# sbt >= 1.3.0 (https://github.com/sbt/sbt/issues/5063)
sbt --supershell=false new sbchapin/quickstart-scala-sbt-native-image.g8
src/main/g8/README.md
Outdated
| @@ -14,15 +13,26 @@ The intention is to use `sbt new` on this library's exterior g8 template, then t | |||
| - `sbt-native-packager` plugin for **building a linked binary** _(builds ahead-of-time using GraalVM statically linking and building all dependencies in a docker image via `sbt graalvm-native-image:packageBin`)_ | |||
| - `scalatest` for **testing** _(default test runner via `sbt test`)_ | |||
| - `sbt-scoverage` for **test coverage** _(statement-level coverage via `sbt coverage`)_ | |||
| - `scalalogging -> slf4j -> log4j2` for **logging** _(scala code -> interface -> mechanism)_ | |||
| - `scalalogging -> logback-classic` for **logging** _(`logback-classic` is used as a backend, but it is very easy to [replace this with something like log4j2](https://github.com/sbchapin/quickstart-scala-sbt.g8/blob/master/src/main/g8/build.sbt#L19))_ | |||
There was a problem hiding this comment.
I would advise against the log4j2 part of this comment due to it adding reflective overhead which GraalVM native-image will take umbrage with (and consequently require additional configuration). It's a good suggestion, but one I would bring in along with instruction and explicit reflective configuration (or alternatively wait for more SubstrateVM support).
There was a problem hiding this comment.
👍 I'll do that, if for nothing else just to keep the verbosity down. If there was a nice way of alluding to your explanation above I'd love a link to it. Maybe a fun facts section?
I know for a fact a few people will be trying to slap log4j2 on this bad boy and it might be nice to save them the trouble.
There was a problem hiding this comment.
I thought about this a tiny bit while I was touching up other parts of the readme. What do you think about this? If you don't like the added verbosity I'd completely understand opting for omitting it altogether.
There was a problem hiding this comment.
I like it!
Good callout that they gonna be trying, and I think we should get in front of that in build.sbt as well - Lord knows they ain't gonna read the readme.
There was a problem hiding this comment.
Lol, too true. I'll drop a quick line in the build.sbt 👍
src/main/g8/README.md
Outdated
| ## Prerequisites | ||
|
|
||
| There are a couple of things you're probably gonna want to have installed on your machine before you go any further. | ||
|
|
||
| - SBT: Used for building, testing and inspecting the project. | ||
| - Install on [OSX](https://www.scala-sbt.org/1.x/docs/Installing-sbt-on-Mac.html) | ||
| - Install on [Windows](https://www.scala-sbt.org/1.x/docs/Installing-sbt-on-Windows.html) | ||
| - Install on [Linux](https://www.scala-sbt.org/1.x/docs/Installing-sbt-on-Linux.html) | ||
| - Docker: Used by the Graal Native Image SBT Plugin (configured in this project's `build.sbt` file) to create a native executable. | ||
| - Install via [Docker Desktop](https://hub.docker.com/?overlay=onboarding) |
src/main/g8/README.md
Outdated
| - Docker: Wraps the entire build process to make your life easier. | ||
| - Install via [Docker Desktop](https://hub.docker.com/?overlay=onboarding) | ||
| - If you only intend to run this project on your own machine (which you can do by [removing this line](https://github.com/sbchapin/quickstart-scala-sbt-native-image.g8/blob/master/src/main/g8/build.sbt#L20)) you won't need Docker so feel free to skip this. |
There was a problem hiding this comment.
This latter bit is env-specific. Technically, you don't need to remove it if you have libc, which would be most linux environments.
Suggested edit:
- Docker: Wraps the entire build process to make your life easier. This means you don't need GraalVM or native-image in your environment.
- Install via [Docker Desktop](https://hub.docker.com/?overlay=onboarding)
- No Docker: If you intend to build this project to a binary on your machine rather than build it within Docker...
- Install [GraalVM CE](https://www.graalvm.org/docs/getting-started/) - or better yet - [install jabba](https://github.com/shyiko/jabba#installation) Java Version Manager to [install GraalVM](https://github.com/shyiko/jabba#usage)
- Install [native-image](https://www.graalvm.org/docs/reference-manual/native-image/) via `gu install native-image`
- [Remove this line](https://github.com/sbchapin/quickstart-scala-sbt-native-image.g8/blob/master/src/main/g8/build.sbt#L20) if you are not running on linux _(and no, OSX is not linux)_There was a problem hiding this comment.
I like this a lot 👍 with the exception of all the negatives on this line:
if you are *not* running on linux _(and *no*, OSX is *not* linux)_
I switched it based on your comment above. Hopefully it looks okay to you?
|
|
||
| ###### How do we use it? ###### | ||
|
|
||
| - Modify `src/main/resource/log4j2.xml` to your desire for local development. It currently has good defaults for moderate log usage in production (20 files rotating, 25 MB max each, INFO level) | ||
| - Modify `src/main/resources/logback.xml` to your desire. It currently has a basic, yet colorful, logger that simply prints TRACE level and above to the console. |
There was a problem hiding this comment.
Good callout that this logger config has too much pumpkin spice.
I'll branch later and add support that Log4j2 was providing in the previous project, but in a better way.
I noticed a few cases of what I think was copy pasta from this project's glorious predecessor. You might not wanna take this whole PR in, but instead use it as a guide for areas in the README that I think could use your attention.
I annotated the more controversial changes in this branch with comments. I'll leave judgement up to you.