Skip to content

deps: Add some overrides to unblock Flutter Color API changes for wide-gamut #920

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

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

chrisbobbe
Copy link
Collaborator

This commit follows the request in
flutter/flutter#154123
by pointing to a Flutter maintainer's fork of flutter_color_model and color_model while we wait for their PR from that fork to be settled.

To validate this change, I checked out the roll from engine-flutter-autoroll that failed on our tests:
flutter/flutter#153985
and confirmed that our tests (the ones that run with tools/check test --all) failed with same-looking errors before this change and passed after this change.

I did run all of tools/check --all, not just the test suite. There was "info"-level output from the analyzer telling us about things we've been using that will be deprecated with the proposed Color API change (Color.value and Color.withOpacity). We'll want to respond promptly to these deprecation notices -- and in fact our CI treats this "info"-level output as an error -- but I think we can leave that out of scope for the customer-tests fix, because we've set it so "info"-level analyzer output isn't fatal there:
flutter/tests@54cc35f9f#diff-d2bef7bb93d58c3db877456685dad3e25d1546c1fc318e9d6c560e91c457eaa7R10

…e-gamut

This commit follows the request in
  flutter/flutter#154123
by pointing to a Flutter maintainer's fork of `flutter_color_model`
and `color_model` while we wait for their PR from that fork to be
settled.

To validate this change, I checked out the roll from
engine-flutter-autoroll that failed on our tests:
  flutter/flutter#153985
and confirmed that our tests (the ones that run with
`tools/check test --all`) failed with same-looking errors before
this change and passed after this change.

I did run all of `tools/check --all`, not just the `test` suite.
There was "info"-level output from the analyzer telling us about
things we've been using that will be deprecated with the proposed
Color API change (`Color.value` and `Color.withOpacity`). We'll want
to respond promptly to these deprecation notices -- and in fact our
CI treats this "info"-level output as an error -- but I think we can
leave that out of scope for the customer-tests fix, because we've
set it so "info"-level analyzer output isn't fatal there:
  flutter/tests@54cc35f9f#diff-d2bef7bb93d58c3db877456685dad3e25d1546c1fc318e9d6c560e91c457eaa7R10
@chrisbobbe chrisbobbe requested a review from PIG208 August 28, 2024 18:57
@chrisbobbe
Copy link
Collaborator Author

(Requesting @PIG208 for a review instead of @gnprice because Greg is on vacation and the deadline on this is before Greg gets back.)

@PIG208
Copy link
Member

PIG208 commented Aug 28, 2024

I think it is concerning that we rely on packages whose maintainer does not seem to be active anymore. Maybe that's a sign that we should eventually drop the dependency after this fix, but I'm not sure how much work that will be.

@PIG208
Copy link
Member

PIG208 commented Aug 28, 2024

Tested and could verify that the fix works. LGTM!

@chrisbobbe
Copy link
Collaborator Author

Thanks for the review! Merging.

I think it is concerning that we rely on packages whose maintainer does not seem to be active anymore. Maybe that's a sign that we should eventually drop the dependency after this fix, but I'm not sure how much work that will be.

Interesting, yeah. I guess a helpful signal for the maintainer's engagement will be to see how quickly they respond to james-alex/color_models#10 . 🙂

@gaaclarke
Copy link

Thanks everyone for moving on this quickly. This is kind of a weird flow, forward fixing things. It tested fine locally so hopefully I got everything right 🤞

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this pull request Aug 31, 2024
And update Flutter's supporting libraries to match.

Also, as anticipated in zulip#920 (see PR description), handle some
deprecations in the Color API from flutter/engine@eff1b76cf (rolled
in flutter/flutter@8c1a93508). Deprecation warnings are "info"-level
in analyzer output, and CI treats that as fatal, because we don't
want to let those hang around.

In particular:

- `withOpacity` is replaced with the new `withValues`

- `value` is deprecated (we did most of the work to handle this in a
  recent commit; here we just add an `ignore:` for one place we're
  still using it).
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this pull request Sep 7, 2024
And update Flutter's supporting libraries to match.

Also, as anticipated in zulip#920 (see PR description), handle some
deprecations in the Color API from flutter/engine@eff1b76cf (rolled
in flutter/flutter@8c1a93508). Deprecation warnings are "info"-level
in analyzer output, and CI treats that as fatal, because we don't
want to let those hang around.

In particular, `withOpacity` is replaced with the new `withValues`.

Also, `value` is newly deprecated, but we migrated off that in the
previous commit.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this pull request Sep 7, 2024
And update Flutter's supporting libraries to match.

Also, as anticipated in zulip#920 (see PR description), handle some
deprecations in the Color API from flutter/engine@eff1b76cf (rolled
in flutter/flutter@8c1a93508). Deprecation warnings are "info"-level
in analyzer output, and CI treats that as fatal, because we don't
want to let those hang around.

In particular, `withOpacity` is replaced with the new `withValues`.

Also, `value` is newly deprecated, but we migrated off that in the
previous commit.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this pull request Sep 7, 2024
And update Flutter's supporting libraries to match.

Also, as anticipated in zulip#920 (see PR description), handle some
deprecations in the Color API from flutter/engine@eff1b76cf (rolled
in flutter/flutter@8c1a93508). Deprecation warnings are "info"-level
in analyzer output, and CI treats that as fatal, because we don't
want to let those hang around.

In particular, `withOpacity` is replaced with the new `withValues`.

Also, `value` is newly deprecated, but we migrated off that in the
previous commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants