Vscode: Broken strict null build should produce a yellow build

[isidorn]

Currently if the strict null checks fails our build completely fails.
Due to this we did not produce a couple of insiders and we seem to be constantly breaking the build.

Thus I recommend that we change that if the strickt null compilation fails we change the build to yellow, not red.
This is just a TFS option and I can also do the change if you agree.

fyi @joaomoreno @bpasero @roblourens

[bpasero]

This is a must for me, I cannot tell how much time I loose per week due to this.

[joaomoreno]

@isidorn @bpasero I'm not too sure I agree with this. The goal is to go to 100% strict null checks and going yellow with builds will just slow down the progress of that goal.

[isidorn]

@joaomoreno I do not agree it will slow down the progress. Fixing this issues usually does not take a lot of time thus people would fix them if they get pinged in the build channel for a yellow build.
Just the build should not be broken, since it actually takes a lot of time to re-trigger the build.

[bpasero]

@joaomoreno the process will not slow down as long as we make sure people get pinged about introducing strict-null issues on the build channel and fixing them is typically not a big deal.

I would argue that we already crossed the line by not running the smoke test anymore as part of the build. To me strict-null is in the same category of annoyance.

[bpasero]

/cc @Microsoft/vscode for input

[bpasero]

Alternatively what I would like to see is yarn watch simply returning errors from strict-null so that I have one place to look at. E.g. by having some smart script that executes both things and scrapes the output for errors from both.

[octref]

I'd love TS to have two channels, one for errors and one for warnings.

• Warning: unused variables, strict null checks, implicit any, etc
• Error: everything else

[mjbvz]

Warnings can tend to be ignored and accumulate. And for strict null, while some of the errors it reports are just silly type annotation issues, it also catches actual programming errors that would have resulted in exceptions.

I’d prefer increasing developer’s awareness of strict null errors before check in. (And we should keep pushing to strict null check the codebase so that our normal build eventually is strict null enabled)

[rebornix]

After porting some projects from JS to Swift (which is strict null checked in nature) and turning on strict null check for GH PR extension, I prefer to see strict null check results as errors. We can Improve the toolchain but not lower the severity level.

[Tyriar]

@mjbvz @rebornix the problem is that it's easy to let a commit in that doesn't commit strictly as it's done on yarn watch, and when this happens it breaks the automated release in the morning which costs time to resolve. Zurich would probably spend much less time just pinging people about build errors than fixing (even if Redmond caused the issue), queueing new builds and re-releasing in their morning.

Of course this is all temporary, once we've finished the migration strict null checks will always be on and it'll be red again when it fails.

[aeschli]

@mjbvz Matt, any chance the TypeScript team can help us so we can avoid having to run a separate build for null checks on a subset of files?

Something like a #begin strict-null-checks=false comment on top of a file. I'm sure it would help other projects too that want to incrementally add null annotations.

[Tyriar]

@aeschli https://github.com/Microsoft/TypeScript/issues/11386, https://github.com/Microsoft/TypeScript/issues/9432#issuecomment-249944516

It's the hardest-to-reach fruit. It's not clear where in the tree this fruit even is, other than that it's really high up.

[mjbvz]

Closing since our normal build is now strict check enabled. Extra build step has been deleted!