Add `sbt-tpolecat` to prevent common bug patterns
See also: https://github.com/typelevel/cats-effect/pull/4054/
~~also update sbt to run with newer JVMs (https://github.com/sbt/sbt/issues/7235)~~
Hello @froth, thanks for your contribution. What a timing! I had created this branch here yesterday evening that fixes many issues in the build, sbt version, GitHub workflow and most lib versions. There are overlaps between our branches, but I'm more prone to open a PR from mine to fix more things at the same time. What I can offer you, if you like the idea is to rework this PR to add this scalacOption but via the addition of sbt-tpolecat to the build, WDYT?
@TonioGela timing indeed :) no worries, I can redo this change onto your code once merged.
I am not sure about sbt-tpolecat for this template however. I am a huge fan of the plugin and use it myself. However in the default configuration it treats warnings as errors. This could lead to frustration for people just wanting to try out cats-effect. They would have to remove all unused imports before trying out their code etc. What do you think?
We could add ThisBuild / tpolecatDefaultOptionsMode := DevMode, but I am not sure whether that sets a good example.
@TonioGela timing indeed :) no worries, I can redo this change onto your branch once merged.
Thanks!
I am not sure about
sbt-tpolecatfor this template however. I am a huge fan of the plugin and use it myself. However in the default configuration it treats warnings as errors. This could lead to frustration for people just wanting to try out cats effect. They would have to remove all unused imports before trying out their code etc. What do you think?
Good point. I still prefer to add sbt-tpolecat as it adds a lot of sane defaults, so what we can do is add it and do one of the following:
- Manually add
ScalacOptions.fatalWarningstotpolecatExcludeOptions - Setup as the default mode the verbose one, which helps developers to debug with more verbose warnings. The downside is that they will have to remember to set the environment variable
SBT_TPOLECAT_CIon their CIs in order to enable fatal warnings again, but this can probably addressed with an inline comment just before setting the default mode in the build, right?
- Setup as the default mode the verbose one
I think the underlying question is: what is the intended audience of this template. People wanting to try ce out or people bootstrapping production code(with CI, etc). As always the answer is "both", but I personally lean more towards the "trying out" usecase (my personal experience is that in $WORK related projects most people build their own templates anyways). Therefore I think that verbose mode and comment is a good compromise.
but I personally lean more towards the "trying out" usecase
For this case there's scala-cli and the typelevel/toolkit :trollface:
Therefore I think that verbose mode and comment is a good compromise.
Okay, let's follow this path momentarily. I will try to ask more people what they think about this possibly new default before merging, as it may be really opinionated.
Once the PR is ready (this one or a new one that you may want to open), we can share the link in Discord and discuss it a bit before making any decision.
Once the PR is ready (this one or a new one that you may want to open), we can share the link in Discord and discuss it a bit before making any decision.
That sounds like a good solution. I will reset this one to preserve our discussion. And I will have an eye on discord :)
@TonioGela updated the PR
My two cents, just add sbt-tpolecat, it makes the management of all those flags pretty easy and Scala version resistant. Also, in general, most of the flags are useful for writing good CE apps. If people find the warnings too cumbersome then they may tweak them. Adding info about the plugin being used and how to manage it, or a link to the plugin docs would be great.
Note however that, I am biased since I always use the plugin and I have never ever bothered to use the "development" mode.
So, a lot of opinions have been expressed on Discord, and it seems that the leading opinion is prompting the user whether to add fatal-warnings or not with sbt-tpolecat always included. @froth do you think you can rework the PR to add this flag maybe?
@TonioGela can do so. I think g8 wants a default for the option. I would use "without fatal" if that is ok
@TonioGela can do so. I think g8 wants a default for the option. I would use "without fatal" if that is ok
I think the "vox populi" is in favor of keeping them enabled :)
@TonioGela can do so. I think g8 wants a default for the option. I would use "without fatal" as default, ok?
Yeah it seems folks would prefer without fatal for newcomers.
Also, does the template generate a README? If so, it would be great to add a note about the plugin there.
Yeah it seems folks would prefer without fatal for newcomers.
Also, does the template generate a
README? If so, it would be great to add a note about the plugin there.
Yes, let's add a README and a note, lgtm
Thanks for the feedback :)
I added a paragraph to the README and made fatal warnings configurabel.