-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
In 2022 this PR - #101101 - made a seemingly simple change: add a couple of extra debug properties to some layer classes. The PR included a test, yay!
Turns out the web implementation had a bug, stringifying TileMode as TileMode.repeated instead of just repeated like mobile does. Since the implementation is in the engine repo the author couldn't fix it in the PR, and instead added an isBrowser check and tested different outputs.
Fast-forward to January 2024. I'm fixing the web bug on the engine - flutter/engine#49786. I didn't know we had a test in the framework testing this. I wrote the engine test, got the PR reviewed, and was mentally preparing to submit. Alas, the framework test failed because it expects a wrong behavior on the web. I guess my PR isn't landing today.
I'm now sending a PR to prepare the framework test - #141731. Alas, I missed another test that also needs to be prepared for the engine change. Send a PR for that too - #141700. Once these land, I can submit (hopefully) the engine PR.
Later, when the dust settles, I must remember to send framework PRs to remove the kIsWeb.
Expected behavior
For such a small change, the original PR from 2022 (#101101), should've been able to just fix everything in one go and one code review.
Actual behavior
Our setup put a barrier in front of the original contributor, pushing them to incur technical debt (probably not realizing it, since the simple isBrowser check doesn't feel too tech debty). The debt persisted until 2024. Now to fix it, I have to send 5 PRs, and request 5 separate code reviews. That seems like an unreasonable amount of effort for simple changes like this.
I feel like we can improve the situation considerably.