Add a --fail-on-severe flag, but keep it as false
#781
Conversation
| onLog ??= stdIOLogListener; | ||
| logListener = Logger.root.onRecord.listen((r) { | ||
| if (r.level == Level.SEVERE) { | ||
| severeLogHandled = true; |
There was a problem hiding this comment.
do we ever reset this (for watch mode)?
There was a problem hiding this comment.
There was a problem hiding this comment.
And again in watch_impl.dart.
|
It might be better to fail the whole build early for this case, although doing so is a bit... tricky so this is probably safer. |
|
Yeah I thought about that, but we don't have a great lifecycle right now. If we can refactor to have a more clear |
| @@ -31,10 +33,14 @@ class BuildOptions { | |||
| // For testing only, skips the build script updates check. | |||
| bool skipBuildScriptCheck; | |||
|
|
|||
| // For internal use only, flipped when a severe log occurred. | |||
| bool severeLogHandled = false; | |||
There was a problem hiding this comment.
This runs counter to the goal in #95 and makes me a little nervous.
Any particular reason we are using this rather than throwing at the point of the log call?
There was a problem hiding this comment.
We can (and should) move the boolean to the BuildImpl class to remove the addition of dynamic state to BuildOptions.
The logs are coming from a stream so unless that becomes synchronous failing in the listener is sketchy.
There was a problem hiding this comment.
Do you want me to revert?
There was a problem hiding this comment.
I'm fine with the change for now. We can revisit in the linked issue.
Work towards #215.
We can flip this to
trueafter better testing/validation this is a good idea.I purposefully didn't update the CHANGELOG/pubspec yet, will do before I merge.