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

fix: Incorrect Dart and Flutter SDKs compatibility range #867

Merged
merged 18 commits into from
May 13, 2023

Conversation

Nidal-Bakir
Copy link
Member

@Nidal-Bakir Nidal-Bakir commented Apr 21, 2023

New Pull Request Checklist

Issue Description

Closes: #872

Approach

Supporting older versions of the framework and language is not ideal because we miss out on new additional features. Additionally, if the Flutter team removes deprecated APIs (as they did in 3.9.x, see issue #861), it can cause problems for us because we still support an older version that had the deprecated API, while also needing to support a newer version that no longer has the API.

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title build: bump the minimum required Dart SDK version build: Bump the minimum required Dart SDK version Apr 21, 2023
@parse-github-assistant
Copy link

parse-github-assistant bot commented Apr 21, 2023

Thanks for opening this pull request!

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

@Nidal-Bakir Nidal-Bakir changed the title build: Bump the minimum required Dart SDK version build: Bump the minimum required Dart SDK version in the Dart package Apr 21, 2023
@Nidal-Bakir Nidal-Bakir added the state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message label Apr 21, 2023
@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (319d4c3) 26.71% compared to head (8877807) 26.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #867   +/-   ##
=======================================
  Coverage   26.71%   26.71%           
=======================================
  Files          47       47           
  Lines        2905     2905           
=======================================
  Hits          776      776           
  Misses       2129     2129           

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

@Nidal-Bakir
Copy link
Member Author

Do not merge until the Changelog entry 5.0.0 is modified to indicate the bump of the min SDK version.
I will edit the Changelog entry 5.0.0 when #860 merged so I can see the entry in this PR

@mtrezza
Copy link
Member

mtrezza commented Apr 22, 2023

What's the purpose of this PR? It doesn't seem to be a breaking change, it's only an upgrade from >=2.12.0 <3.0.0 to
>=2.17.0 <3.0.0.

@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented Apr 22, 2023

See the discussion

If we remove a property and therefore remove support for currently supported Flutter SDK versions then this is a breaking change. Let's discuss this in the issue

With this version upgrade, we will no longer support Dart versions older than 2.17.0.

@mbfakourii
Copy link
Member

@Nidal-Bakir
I don't think it is a breaking change either
Are you sure about this?

@mtrezza
Copy link
Member

mtrezza commented May 7, 2023

@Nidal-Bakir You are correct with your comment that changing the lower bound would be a breaking change. But what is the purpose of this PR, why this lower bound change? Could you add a description to the PR?

@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented May 7, 2023

But what is the purpose of this PR

I'm using a pull request to make this change because I don't see a good place to put it. We cannot make such a change with other pull requests. For example, we cannot bump the minimum required Dart SDK in this pull request (#860) because it would not be noticeable in the commit log. Therefore, I opened a new pull request specifically for this bump so that we can see it in the commit log

why this lower bound change?

Please read this #861 (comment) we have a discussion about it earlier.

Could you add a description to the PR?

You are right, I should provide a description for the pull request. I will add it right away.

@Nidal-Bakir
Copy link
Member Author

I don't think it is a breaking change either
Are you sure about this?

#861 (comment)
Please read the comment I am referring to, as well as the comments that follow it.

@mtrezza
Copy link
Member

mtrezza commented May 8, 2023

I remember now, thanks. I think we need to find a strategy for when remove support for a dart or flutter version.

Usually we follow the long-term support schedule of the framework publisher, but I couldn't find any infos about that for Dart / Flutter.

Otherwise we are choosing an arbitrary date for when to drop support for a version and need a discussion every time we do that, which shouldn't be necessary.

@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented May 8, 2023

I remember now, thanks.

You are most welcome

I think we need to find a strategy for when remove support for a dart or flutter version.

Basically, we do not need any special strategy for bumping the Dart SDK constraint. We simply bump the constraint like any other package. For example, take a look at this PR on the flutter-fire repository: feat: bump dart SDK constraint to 2.18. They did not even count it as a breaking change because, in the Flutter ecosystem, we do not have the concept of LTS or worry about any app that uses a Dart version older than the current stable one. This is because you should have migrated to a newer version a long time ago.

Usually we follow the long-term support schedule of the framework publisher, but I couldn't find any infos about that for Dart / Flutter.

Flutter does not have the concept of LTS you should always migrate to the latest stable release

@mtrezza
Copy link
Member

mtrezza commented May 9, 2023

I've opened #872 to discuss this in more detail.

@mtrezza mtrezza changed the title build: Bump the minimum required Dart SDK version in the Dart package fix: Incorrect Dart SDK upper version bound; bump minimum required Dart SDK version May 10, 2023
@mtrezza
Copy link
Member

mtrezza commented May 12, 2023

  • Removed the Dart <2.18 jobs from the CI, since this PR will set the Parse Dart SDK to require >= 2.18
  • Updated the SDK range to >= 2.18 <3.1.0

@mtrezza mtrezza changed the title fix: Incorrect Dart SDK upper version bound; bump minimum required Dart SDK version fix: Incorrect Dart SDK compatibility range May 12, 2023
@mtrezza
Copy link
Member

mtrezza commented May 12, 2023

I've added a flutter range, and the CI failed with:

Package validation found the following potential issue:

  • The Flutter constraint should not have an upper bound.
    In your pubspec.yaml the constraint is currently >=3.3.0 <3.11.0.

    You can replace that with just the lower bound: >=3.3.0.

That's a truly interesting concept.

@mtrezza
Copy link
Member

mtrezza commented May 12, 2023

Only Flutter 3.10 fails with:

info • 'window' is deprecated and shouldn't be used. Look up the current FlutterView from the context via View.of(context) or consult the PlatformDispatcher directly instead. Deprecated to prepare for the upcoming multi-window support. This feature was deprecated after v3.7.0-32.0.pre • packages/flutter/lib/parse_server_sdk_flutter.dart:81:32 • deprecated_member_use
error • The named parameter 'vsync' isn't defined • packages/flutter/lib/src/utils/parse_live_list.dart:276:9 • undefined_named_parameter

What would be the next steps?

  1. merge this and release Dart 5.0.0
  2. fix the vsync issue and release Flutter 5.0.0

I believe @Nidal-Bakir mentioned that Flutter 3.10 will require Dart >= 2.19, is that still the case and if yes, what can we do about this, since this PR currently only will require >2.18.0

@mtrezza mtrezza changed the title fix: Incorrect Dart SDK compatibility range fix: Incorrect Dart / Flutter SDK compatibility range May 12, 2023
@mtrezza mtrezza changed the title fix: Incorrect Dart / Flutter SDK compatibility range fix: Incorrect Dart SDK compatibility range May 12, 2023
@Nidal-Bakir Nidal-Bakir changed the title fix: Incorrect Dart SDK compatibility range fix: Incorrect Dart and Flutter SDKs compatibility range; move the example folder to the root May 12, 2023
@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented May 12, 2023

The Dart SDK is ready. We need to merge the following PRs to release version 5.0.0:

Please note that neither of these pull requests includes a proper statement about Dart 5.0.0 in the changelog.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Could you add a changelog entry?

@Nidal-Bakir
Copy link
Member Author

Could you add a changelog entry?

Sorry, but I am a bit confused about what should I put it. Should we merge this #860 first and then update the entry?

Since we need 2 PRs to release Dart 5.0.0.

#860 (review)

  • Since this won't be released, there is no version 5.0.0, so it also can't be in the changelog yet.
  • Reformatted the BREAKING CHANGES section to align with org-wide style.

@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

We don't have auto-release yet, which would generate a changelog entry for each PR according to the commit message (=PR title). So what we do instead until when is with each PR add a changelog entry for the PR, just like it would be auto-generated.

So currently, the changelog entry would be the PR title, and if necessary - in addition - an entry in the BREAKING CHANGES section. Since we have the Unreleased line at the top of the changelog entry, it is clear that the entries underneath are not of a released version yet.

@mtrezza mtrezza changed the title fix: Incorrect Dart and Flutter SDKs compatibility range; move the example folder to the root fix: Incorrect Dart and Flutter SDKs compatibility range May 13, 2023
@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

I've added the changelog entries for dart and flutter. This is kind of a rarity, as we have 1 PR that creates a changelog entry in both packages; I think that usually wouldn't work this way with auto release, but we can do it here. I don't mention the moving of the example projects, as they would normally be a refactor or docs commit and not appear in the changelog - they have no effect on the SDK itself.

@Nidal-Bakir
Copy link
Member Author

Thanks for clarifying.

@mtrezza
Copy link
Member

mtrezza commented May 13, 2023

I think we can merge this?

@Nidal-Bakir
Copy link
Member Author

Yes, we are ready.

@mtrezza mtrezza merged commit 2e133ff into parse-community:master May 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Dart / Flutter framework support policy
3 participants