Repository navigation
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-team-archive/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_builderto account for the changes. - Both
go_routerandgo_router_builderhave 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).
Activity
johnpryan commentedon Jul 26, 2022
It might be possible to fix go_router_builder in a backwards compatible way, see https://github.com/flutter/packages/pull/2381/files#r930121119
chunhtai commentedon Jul 26, 2022
we could have a github bot that only flags the pr but don't block it from merging. I saw we do update some meta data for all packages in the flutter/package from time to time. those will be an annoying false positive
stuartmorgan-g commentedon Jul 26, 2022
This suggestion isn't about the specific PR, it's about the long-term maintenance of the packages. These packages haven't existed for all that long and this has already been a problem at least twice. It's likely to happen again.
stuartmorgan-g commentedon Jul 26, 2022
The federated plugin tooling has a heuristic for that based on the set of packages being touched. In this case, what we'd want is to block/flag/whatever the case where exactly these two packages are being touched. It's very unlikely that someone would make dangerous intertwined PR for these two packages that also included changes to some other random package(s), so it's a low-risk filter.