Skip to content

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented May 10, 2019

Description

Prefer using precisionErrorTolerance rather than random hardcoded epsilons when we need an epsilon. Also prefer moreOrLessEquals (which uses precisionErrorTolerance by default) rather than closeTo.

Tests

I added the following tests:

Updated existing tests.

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 Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@dnfield
Copy link
Contributor Author

dnfield commented May 10, 2019

Some of the updated tests appear to fail on windows/Linux but pass on macOS...

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the same thing when I added some tests like this, where the Mac tests would fail due to being off by slightly too much. I tried to prefer moreOrLessEquals but found that the threshold was too small for the Mac tests. All other tests worked though, including locally, and I couldn't reproduce the Mac failure locally. Not sure what's up.

Anyway, I'm all for standardizing and simplifying error tolerance like this. I'm hoping that the tests pass and you don't have to remove more tests.

LGTM 👍

@dnfield dnfield merged commit 382704c into flutter:master May 10, 2019
@dnfield dnfield deleted the epsilons branch May 10, 2019 20:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants