-
Notifications
You must be signed in to change notification settings - Fork 29.3k
Description
Capturing from Discord discussion in the wake of #108274 and flutter/packages#2381 (see also #89518 for background, which was the solution to the same basic problem but with federated plugins in flutter/plugins).
The immediate cause of #108274 was that the CI didn't catch it because of a tooling bug (see #108274 (comment), and the fix for that in flutter/plugins#6146). However, based on past experience and Discord discussion, there's a non-trivial chance that if that bug hadn't existed, the flow would instead have been:
- Presubmit fails on the initial version of the PR.
- The PR is "fixed" to also update
go_router_builder
to account for the changes. - Both
go_router
andgo_router_builder
have new versions published.
This would have almost certainly broken fewer people (including not breaking flutter/packages PRs), but is actually still wrong. Step 2 is incorrect because it violates semver: go_router
4.2.1 would still have been published with changes that would break existing versions of go_router_builder
, and there's nothing that requires that someone who updates go_router
also update go_router_builder
if the constraints don't say it's required (which they would not have here). And it would also be broken the other way: someone updating go_router_builder
but not go_router
would have been broken, because go_router_builder
couldn't have expressed that it required >= 4.2.1 because 4.2.1 wouldn't be published to depend on.
The safest way to avoid that would be to prevent changes to go_router_builder
in the same PR that changes anything in go_router
's lib/
, as we've done with federated plugins. I'm not sure how though for this pair how often this would result in false-positives that would be problematic (e.g., you want to update code comments in one package while making actual changes to the other).