log scalafix --check diffs as errors via a sbt logger
#106
Conversation
| import sbt.{Level, Logger} | ||
|
|
||
| /** Split an OutputStream into messages and feed them to a given logger at a specified level. Not thread-safe. */ | ||
| class LoggingOutputStream( |
There was a problem hiding this comment.
Surprisingly, I couldn't find any helper class in sbt... The best I found was https://github.com/sbt/util/blob/d31b9c509384ecaf37f2ffaec8731250e3c60532/internal/util-logging/src/main/scala/sbt/internal/util/LoggerWriter.scala, which is internal and doesn't match the types we need in that case.
| @@ -140,6 +141,8 @@ object ScalafixPlugin extends AutoPlugin { | |||
| loadedRules = () => scalafixArgs().availableRules().asScala, | |||
| terminalWidth = Some(JLineAccess.terminalWidth) | |||
| ).parser.parsed | |||
| val errorLogger = | |||
| new PrintStream(LoggingOutputStream(streams.value.log, Level.Error)) | |||
There was a problem hiding this comment.
I thought about deferring the logging until the point we know the status code, to adjust the level accordingly, but looking at the usage of that printStream, I think it's reasonnable to always log as error, except maybe for --stdout, but that's a lot of custom code for something that I don't see being used through sbt
| val taskStreams = (example / Compile / scalafix / streams).value | ||
| val reader = taskStreams.readText(taskStreams.key) |
There was a problem hiding this comment.
The lastLogFile used in a few scripted did not work as it seems that the except failure just before this gets called flush out the output... In the end, this looks more readable IMHO.
bjaglin
force-pushed
the
sbt-logger
branch
3 times, most recently
from
5cc3df3 to
7ed121c
Compare
|
The scala 2.10 / sbt 0.13.x build has been failing a few times because of network issues, such as I'll retry later. |
There was a problem hiding this comment.
Thank you for this contribution! This looks great, no objections from me 👍
Re the 0.13.x failure. I would be OK with dropping sbt 0.13.x support. Most of the sbt plugin ecosystem has moved on from 0.13 and the users on 0.13 can continue to use older sbt-scalafix versions.
In that case it's just a transient infrastructure issue (bintray? I have seen it today on other scripted tests from other projects), but it did take quite a lot of effort to support 0.13.x in #102. Now that it's done, we might as well continue for a while, and revisit the decision next time it's a hassle I guess. |
Diffs were dumped on stdout. When scalafix is run in parallel with many other tasks across multiple sub-projects, this makes it much easier to find out which files are the culprits and why, as the logs stand out and are available via `sbt last scalafix`
Sounds good 👍 |
|
The infra failure was actually hiding flakiness in the new test case (due to the test protocol, not the implementation), particularly on sbt 0.13.x & windows, but with 13efb64 it's finally green. |
|
@olafurpg should I go ahead and merge this then? I am wondering if I may, but other PRs also look like easy merges. I am willing to help out maintaining this repo in the foreseeable future. |
Diffs were dumped on stdout. When scalafix is run in parallel with many other tasks across multiple sub-projects, this makes it much easier to find out which files are the culprits and why, as the logs stand out and are available via
sbt last scalafix.Lint errors are already logged properly via https://github.com/scalacenter/sbt-scalafix/blob/v0.9.15-2/src/main/scala/scalafix/internal/sbt/ScalafixLogger.scala (and are not dumped to the
printStreamas far as I can tell, so we don't get duplicates).