Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue URL: N/A
What is the current behavior?
One matrix job runs every check on Windows and macOS. Several of those read files and report a verdict that cannot vary by operating system, so they run twice for the same answer.
From a recent run, Windows / macOS: Spell Check 27s / 13s, Typecheck 4s / 2s, Test 2s / 1s. macOS bills at 10x Linux, Windows at 2x.
There is also no Linux runner anywhere, while Vercel builds on Linux. Both runners have case-insensitive filesystems, so an import with the wrong casing passes CI and fails production.
What is the new behavior?
Four jobs:
npm ci, lint, tests and spellcheck before reporting, and it stops annotating every finding twice.Also:
cache: npmon both installing jobs,NODE_VERSIONdeclared once at the top, and the matrix job renamed fromtest, which no longer ran any.Lint stays in the matrix deliberately. Prettier rewrites line endings and Check Diff catches it, so it is the one check that genuinely differs by OS. It is also the most expensive step at 59s / 32s.
Now that the admonition check has its own job,
fetch-depth: 0on its checkout replaces the targetedgit fetchit used to do. That workaround existed to avoid pulling history for a job full of unrelated steps, which no longer applies.Does this introduce a breaking change?
Other information
This needs a branch protection change the moment it merges, or merges break.
mainandmajor-10.0currently requireTest on macOS-latestandTest on windows-latest. Those job names no longer exist, so both branches will be waiting on checks that never report.Switch both to requiring
Verifyalone. It fails if any of the three fail, and it keeps working if the matrix changes later, which is why it exists.translation/jphas no required checks, so nothing to do there.One ordering note: GitHub's UI only offers names it has seen, so
Verifybecomes selectable after this runs once. Worth having someone with admin ready before merging.The
Verifypattern is borrowed from ionic-framework'sverify-*jobs, which exist for the same reason.The two
Test on ...checks will never report: this PR renames those jobs, and branch protection still requires the old names. Updating it to requireVerifyinstead clears them.