-
Notifications
You must be signed in to change notification settings - Fork 29.3k
explicitly set packageConfigPath for strategy providers #163080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
Awesome! I'd like to land a test somewhere for this so we don't break it in the future accidentally, but it's hard to tell what to test for. The aforementioned bug mentions error logs (which I guess we could test for, but that feels like a brittle/bad test) and breakpoint issues. I wonder if we could create a test in DWDS to check whether breakpoints in a synthesized monorepo are working as intended. There are also some tests in this repo that test different Anyways, LGTM and we can maybe think of a test separately. |
FYI - I believe the way to autosubmit is to use the label |
Ah! I see. Thanks for the info. I'll keep that in mind next time :) |
Thanks for the fix! Any estimate in what release / timeline this will land in? Debugging web in workspaces continues to be broken, making life a little harder. :) |
This is in the most recent Flutter 3.31 beta. Stable release should be roughly 3 months after the last one, so this should be in a stable sometime in May. |
Thank you for the timeline info. I personally find it difficult without a working debugger. I suppose I can switch to the beta. I suppose there have been insufficient complaints to warrant a cherry pick? |
Yeah I haven't seen enough traffic, but I also believe it would be a complicated cherry-pick since it relies on a new version of DWDS, which potentially could bring in multiple other issues. |
24.3.5
fixes #161469