Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@srujzs
Copy link
Contributor

@srujzs srujzs commented Aug 18, 2020

Changes in processing compatibility info in dart:html requires
these getters to be null-checked.

Description

CanvasElement attributes will be changed to be potentially nullable (see https://dart-review.googlesource.com/c/sdk/+/155844), so they are null-checked.

Related Issues

No new issue for this, but related to the original process in dart-lang/sdk#41905.

Tests

No tests added.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [contributor guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [C++, Objective-C, Java style guides] for the engine.
  • I read the [tree hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [handling breaking changes].

  • No, no existing tests failed, so this is not a breaking change.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@auto-assign auto-assign bot requested a review from gaaclarke August 18, 2020 00:32
@gaaclarke
Copy link
Member

Check out the "format_and_dart_test" Looks like you are using non-nullable features in code that isn't compiling with them.

@srujzs
Copy link
Contributor Author

srujzs commented Aug 18, 2020

This is strange. In my local build, when I add the aforementioned Dart SDK patch to third_party and attempt to felt build without this CL and enforcing null-check lints, I see null-check related errors e.g. Error: Operator '*' cannot be called on 'int?' because it is potentially null. I'm not sure if the Flutter try bots are configured improperly on the Dart SDK, but I'm seeing similar errors: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8872137059857262464/+/steps/build_host_debug/0/stdout

Although, now when I run felt test rebased to the latest, I'm seeing the same error as https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8871672762064432336/+/steps/felt_ios-safari_test/0/stdout

@gaaclarke
Copy link
Member

Paging @yjbanov, he knows more about web_ui and the current state of the dart null safety code.

Changes in processing compatibility info in dart:html requires
these getters to be null-checked.
@yjbanov
Copy link
Contributor

yjbanov commented Aug 20, 2020

Windows host engine failure looks unrelated (infra flake even). Merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants