8 Replies
OK, so find some good lessons we should align across the team on this evening.
In this particular case, the Playwright PR included a change to the root-level
package.json which removed a dependency needed by the Netlify Function @EbonyL-tbd stood up for the Calendar.
https://github.com/TBD54566975/developer.tbd.website/commit/73cfd7b93c095b1d2895542ea7bb68d6b86a160b#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L25
Simple enough. Even running pnpm test or pnpm build locally wouldn't have shown anyone this problem; it only mattered when Netlify went to build the Function.
But! PRs do have a check for Netlify deploy previews:
So what happens when we merge a PR from one of these branches into
main is that the production build breaks. That happened on 15 Novemeber.
And then after that, as a team we continued to merge several PRs in commits after that, each with failing build.
So I wanna flag this because we as a team should all have the policy: when a build is broken, our world stops. This becomes the priority issue to resolve.
The quickest way to do this is usually by reverting the last commit to main and then handling its again once the build resumes working. Once the build breaks, we lose our ability to test and our ability to update the site.
So TL;DR from here out:
* Always ensure the tests, deploy previews, and other checks on a PR are â
before merging to main
* Exception: you can all ignore the BuildKite checks now, for reasons we've discussed in Discord. đ That one we're not relying on yet as we're trialing the product and still mucking around with config. đ
* If a failed build hits main, we immediately revert the commit with the offending change and handle it later.
* And that way, we keep each other unblocked and we're always able to update the site.
Cool cool? đ Emoji acks pls, and let's talk about it here if there's anything in the above we shouldn't all adopt as a team!
Thanks, all -- have an awesome start to your week!Unknown Userâ˘13mo ago
Message Not Public
Sign In & Join Server To View
And thanks for flagging @techgirl1908 - especially while youâve been traveling. Welcome back!
I put some branch protection rules on
main to help us with this, including checks that will block merging if Netlify Deploy preview or Tests fail:
If we need to relax these we can; ping me if I've set things too aggressively.
Thanks for catching this, and I agree!
Unknown Userâ˘13mo ago
Message Not Public
Sign In & Join Server To View