Oh that's interesting. It's possible there are cases where disabled-by-default is the better choice. What do you see as the main trade offs? Obviously it's important that we're testing the default configuration - so either way, we shouldn't be relying on finch to change behavior except in the emergency situation (or intentional A/B tests of course).
I was mainly thinking that for Runtime Enabled Features, we have the status mechanism that biases towards status=stable. I suppose breaking changes are more ambiguous. Eg. if we're removing an API that still has a Runtime Enabled Feature, then setting it back to no status to unship it could be a reasonable option. But I'd worry about the risk of confusion there. Eg. if someone sets it to status=test or status=experimental after it's gone to Chrome Stable with status=stable, there's risk of breaking changes showing up in official builds but not in a lot of chromium developer configurations that have --enable-experimental-web-platform-features set. I don't think that usage is a good fit for how Runtime Enabled Features are designed and communicated. To me behavior changes of any type go through a flow of in-development -> under test -> experimental -> stable (with killswitch) -> mature (no flags). Once we hit the 'stable' feature in stable release for some period of time, I don't think there's really any going back - only the possibility of a new breaking change following this path. I guess this mirrors our launch process as well. You don't need an I2D&R to flip the killswitch on a new API in the first week of being on stable. But before long (maybe a couple weeks?) you cross some threshold where any change or disabling is to a launched feature and needs an I2D&R, and so logically a new Runtime Enabled Feature working up to being on-by-default. Perhaps we should be more thoughtful and explicit about this flow? Rick On Tue, Apr 4, 2023 at 2:24 PM Charles Harrison <[email protected]> wrote: > Thanks Rick this is really helpful. I have been involved in a few changes > where API OWNERS said "flag-gated" and the implementer chose to use a > disabled-by-default flag which I didn't push back on. I think just I needed > this recalibration :) > > On Tue, Apr 4, 2023 at 1:16 PM Rick Byers <[email protected]> wrote: > >> Thanks Charlie. Yes I should have been specific, I'm really talking about >> enabled-by-default flags here (eg. Runtime Enabled Feature with >> status=stable). >> >> Note that our process for flipping a kill switch via finch includes first >> landing a CL that disables the flag (confirming it doesn't break any tests >> etc.). We're still learning how best to communicate these rare situations, >> but that'll be an ongoing process. In general I've certainly found it >> helpful when folks update their I2S threads with rollout status whenever it >> deviates from the original plan. >> >> Rick >> >> On Wed, Mar 29, 2023 at 3:49 PM Charles Harrison <[email protected]> >> wrote: >> >>> Thanks for posting this Rick. One call-out that I think is worth >>> mentioning is the distinction between enabled-by-default flags and >>> disabled-by-default flags. The general guidelines for choosing between >>> these is in the links you posted, but given that it can be sometimes a >>> subjective call it might be worth clarifying in e.g. API owners feedback on >>> a particular I2S. I often see requests to flag guard a feature but >>> sometimes it's ambiguous in which direction. >>> >>> On Wed, Mar 29, 2023 at 1:53 PM Rick Byers <[email protected]> wrote: >>> >>>> Hey blink-dev, >>>> >>>> Within Google we’ve had a lot of discussion over the past year about >>>> how we should make increasing use of flags and kill switches >>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/flag_guarding_guidelines.md> >>>> to reduce the risk of breaking changes, but I realized there hasn’t been >>>> discussion here on blink-dev. >>>> >>>> Once a change hits stable it’s quite expensive and time-consuming to do >>>> an emergency patch, but much faster and cheaper to update a server config. >>>> For Google Chrome this means “Finch”, but other big chromium embedders like >>>> Edge also have their own system for setting flags dynamically. So >>>> increasingly we want to ensure that changes which might reasonably cause a >>>> regression are guarded by a flag wherever practical. >>>> >>>> For Blink this simply means using a Runtime Enabled Feature >>>> <https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/renderer/platform/RuntimeEnabledFeatures.md#:~:text=Features%20that%20are%20hidden%20behind,To%20Ship%20has%20been%20approved.> >>>> (or other base::Feature). I think we’re all largely in the habit of using >>>> this for new APIs, but we should also be using one for all intentional >>>> breaking changes and this is something the API owners will be asking about. >>>> >>>> See Flag Guarding Guidelines >>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/flag_guarding_guidelines.md> >>>> for more details and feel free to reach out to me and/or >>>> [email protected] with questions or concerns. >>>> >>>> Thanks, >>>> >>>> Rick >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "blink-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY9iR-n102RPZWfqpUGjYm9W2LpFFb5%2BodimGFUC8Dv8ZA%40mail.gmail.com >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY9iR-n102RPZWfqpUGjYm9W2LpFFb5%2BodimGFUC8Dv8ZA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY_7T%2B%3DN15qiwkoCtJrDPKCcO8FeNvWkTwXLnv7W1QMBZw%40mail.gmail.com.
