brimdata / zed

A novel data lake based on super-structured data
https://zed.brimdata.io/
BSD 3-Clause "New" or "Revised" License
1.34k stars 67 forks source link

Skip notify-downstream if PR has appropriate label #5115

Closed philrz closed 2 months ago

philrz commented 2 months ago

Right now whenever a PR merges in the Zed repo the "notify downstream" automation makes sure both the Zui and Brimcap repos run their CI and update their pointers to the new Zed commit. However, once in a while there's changes in Zed that won't have a functional impact on either of those projects, e.g., Zed docs updates or changes to Zed's own test automation. Two currently open Zed PRs that I think fit this criteria are #5091 and #5095.

We can perhaps rationalize these occasional "unnecessary" CI runs, e.g., repeated runs may help sniff out intermittent bugs. OTOH there are down sides as well, since it creates more notification chatter on Slack/email even for successful runs, and sometimes we'll have to react to CI failures that have nothing to do with our code, e.g., Actions infrastructure problems. These things have a cost, albeit a small one.

The Windows code signing changes for Zui in https://github.com/brimdata/zui/pull/3050 provide yet another motivation to perhaps avoid excess downstream CI runs when possible. The entry-level eSigner plan we use is $20/mo for 20 signings and $1/each for additional signing. We build (and hence sign) a Zui Insiders release each time there's a change in Zui, and this is something I'd like to continue doing because at times we've benefitted from community feedback on the daily builds. Advances of the Zed pointer represent a material change in Zui, so right now on any day when we've only seen Zed changes that don't affect functionality, the automation still puts out a Zui Insiders release. If we avoid some unnecessary advances of the Zed pointer, we might be able to avoid hitting the included 20-signings-per-month eSigner limit and/or avoid paying for more of the $1/each additional and save a few bucks.

To implement the change, here I'm re-using logic that already exists for the skip-autoperf label that I often apply to open PRs to avoid AWS spend running perf tests when we already know the commit will have zero impact on Zed performance. Indeed, I expect I'd probably end up applying the newly-proposed skip-notify-downstream label to most of the same PRs that I currently tag with skip-autoperf, but since there might be times we want to skip one but not the other, I'm proposing separate labels.

Assuming this merges, nobody would have to feel compelled to think about this if they don't want to. It's just a mini cost optimization I'm happy to apply when I think of it to help us save a few bucks and maybe avoid some unnecessary notification chatter.