Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: Remove deprecated property window from dart:ui in favor of PlatformDispatcher #878

Merged
merged 3 commits into from
May 13, 2023

Conversation

Nidal-Bakir
Copy link
Member

New Pull Request Checklist

Issue Description

Closes: #877

Approach

Use the PlatformDispatcher class as recommended by the Flutter docs see window class docs

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry

@parse-github-assistant
Copy link

parse-github-assistant bot commented May 13, 2023

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@codecov
Copy link

codecov bot commented May 13, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.02 ⚠️

Comparison is base (518f768) 38.69% compared to head (1e982e9) 38.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #878      +/-   ##
==========================================
- Coverage   38.69%   38.68%   -0.02%     
==========================================
  Files          56       56              
  Lines        3277     3278       +1     
==========================================
  Hits         1268     1268              
- Misses       2009     2010       +1     
Impacted Files Coverage Δ
packages/flutter/lib/parse_server_sdk_flutter.dart 8.62% <50.00%> (-0.16%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

Is this a breaking change or only an internal refactor without any effects for developers?

@Nidal-Bakir
Copy link
Member Author

Internal refactor the developers will not notice it

@mtrezza mtrezza changed the title fix: Use PlatformDispatcher rather than the deprecated property window form dart:ui fix: Remove deprecated property window from dart:ui in favor of PlatformDispatcher May 13, 2023
@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

Alright, so we also don't need a changelog entry for it. All looks green - yay! - can we merge this?

@mtrezza mtrezza changed the title fix: Remove deprecated property window from dart:ui in favor of PlatformDispatcher refactor: Remove deprecated property window from dart:ui in favor of PlatformDispatcher May 13, 2023
@Nidal-Bakir
Copy link
Member Author

sure, we can!!!!!!!! finally :)

@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

Is this the last breaking change for the Parse Flutter SDK? If so, we could release Flutter 5.0.0 with this PR.

  • Why do we need feat: Upgrade parse_server_sdk to 5.x.x #868 if all checks are green in this PR already? That would be another breaking change we'd probably have to include before a Flutter 5 release.
  • The Dart SDK has a version constant const String keySdkVersion = '5.0.0'; in parse_constants.dart that needs to be kept in sync manually with the version in pubspec.yaml. Does the Flutter SDK have something similar?

@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented May 13, 2023

Why do we need #868 if all checks are green in this PR already? That would be another breaking change we'd probably have to include before a Flutter 5 release.

But then if anyone uses Flutter SDK will be locked to Dart SDK 4.x.x.
And will not see any updates on the Dart SDK. because the Flutter SDK constraint it to <5.0.0

The Dart SDK has a version constant const String keySdkVersion = '5.0.0'; in parse_constants.dart that needs to be kept in sync manually with the version in pubspec.yaml. Does the Flutter SDK have something similar?

No, there is nothing like that in the Flutter SDK

@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

Alright, I'll go ahead and merge.

@mtrezza mtrezza merged commit 9ca8721 into parse-community:master May 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated property window from dart:ui
3 participants