chore(build): Only suppress expected warnings from closure-make-deps #6350
Conversation
cpcallen
added
component: build process
PR: chore
| !(/Missing type declaration./.test(line) || | ||
| /illegal use of unknown JSDoc tag/.test(line))) |
There was a problem hiding this comment.
Do you think we'll be dealing with this long enough that it would be useful to separate these out into an array?
There was a problem hiding this comment.
I sincerely hope not. But when I find myself a fourth regexp to this conditional then it will be time to reevaluate…
| } | ||
| }); | ||
| })).then(() => { | ||
| done(); |
There was a problem hiding this comment.
Is this fixing the prepare error?
There was a problem hiding this comment.
Not as far as I know!
The switch to Promises is to allow the non-Sync version of exec to be used (which in turn is so that we can capture stderr even when the child process does exit(0)). We still need to make sure done() is called after all the rest of the work is done, though, as it was in the original version. (We could of course alternatively return a vinyl stream, but that doesn't seem applicable for this task.)
3 participants
The basics
npm run formatandnpm run lintThe details
Proposed Changes
Modify
buildDepsgulp target (inbuild_tasks.js) to only suppress expected warnings fromclosure-make-deps.Behaviour Before Change
All
stderroutput fromclosure-make-depsredirected to/dev/nullBehaviour After Change
stderroutput fromclosure-make-depsread by the gulp task and logged to console after filtering known-Reason for Changes
While preparing PR #6337, @BeksOmega discovered that our block tests (in
tests/mocha/blocks/) were not being run. This was due to invalidimports, where several of those files attempted to import from../../build/…instead of../../../build/…. I was surprised that this did not provoke more complaints from various parts of our build/test infrastructure, and it occurred to me thatclosure-make-depsmay have discovered the problem and reported it but we threw the error in the bit-bucket along with all the actually-spurious warnings it generates.Thus I modified
buildDepsto be more selective about what is suppresses.Additional Information
Alas it turns out that
closure-make-depsdoes not in fact care if you attempt toimporta file that does not actually exist. Still, it is probably better that we be less heavy-handed in suppressing its warnings generally, so it seems worthwhile keeping the change I made.