Skip to content
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

feat: Add support for Dart 3.1, remove support for Dart 2.18 #969

Merged
merged 51 commits into from Oct 16, 2023
Merged

feat: Add support for Dart 3.1, remove support for Dart 2.18 #969

merged 51 commits into from Oct 16, 2023

Conversation

mbfakourii
Copy link
Member

Pull Request

Issue

Closes: #968

Approach

Improve support policy and CI

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)

@parse-github-assistant
Copy link

Thanks for opening this pull request!

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1ae0808) 39.60% compared to head (f1ce3b7) 39.60%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #969   +/-   ##
=======================================
  Coverage   39.60%   39.60%           
=======================================
  Files          60       60           
  Lines        3333     3333           
=======================================
  Hits         1320     1320           
  Misses       2013     2013           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fix lower bound
fix WillPopScope
@mbfakourii mbfakourii requested a review from a team October 3, 2023 10:36
.github/workflows/ci.yml Outdated Show resolved Hide resolved
packages/dart/README.md Outdated Show resolved Hide resolved
fix code formatting
upgrade examples environment version
packages/dart/README.md Outdated Show resolved Hide resolved
packages/flutter/README.md Outdated Show resolved Hide resolved
packages/dart/README.md Outdated Show resolved Hide resolved
packages/dart/README.md Outdated Show resolved Hide resolved
@mbfakourii
Copy link
Member Author

mbfakourii commented Oct 12, 2023

Not sure this would be possible. These are major versions and there may be breaking changes. We already have a range with parse_server_sdk: ^5.1.2, the flutter SDK just needs to use the published version; not the local version.

We currently do not use the local version.
The reason for the CI error is related to the remove of support from version 2.18

The error in Flutter 3.3 is as follows:

ERR : The current Dart SDK version is 2.18.6.
    | 
    | Because parse_server_sdk requires SDK version >=2.19.6 <4.0.0, version solving failed.

We removed 2.18.6 and it is not supported in Flutter 3.3

I think there is no problem !

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

How is this possible if we did not actually remove Dart 2.18 support yet? It's only removed in this PR, not in the published Parse Dart SDK version.

@mbfakourii
Copy link
Member Author

mbfakourii commented Oct 12, 2023

I think the problem was found !
The issue here is that we are checking the Parse Dart package in the Flutter section of CI.

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

Makes sense, can we remove this line from the CI?

@mbfakourii
Copy link
Member Author

mbfakourii commented Oct 12, 2023

Makes sense, can we remove this line from the CI?

I think to remove this line, it is better to do PR flutter.

What is your opinion about the separation of two CI (flutter and dart)?

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

Sure, so should the line be removed in #971 (review)?

@mbfakourii
Copy link
Member Author

@mtrezza

What do you think about melos?

@mtrezza
Copy link
Member

mtrezza commented Oct 14, 2023

I think you already identified the solution in #969 (comment). If you just remove the checking in the flutter section of the CI in the #971 (review) PR, then all these changes of splitting up the CI are unnecessary. We also want to keep the CI files as uniform as possible across repositories, for easier maintenance, so creating 2 files here if not necessary should be avoided if possible.

@mbfakourii
Copy link
Member Author

I think you already identified the solution in #969 (comment). If you just remove the checking in the flutter section of the CI in the #971 (review) PR, then all these changes of splitting up the CI are unnecessary. We also want to keep the CI files as uniform as possible across repositories, for easier maintenance, so creating 2 files here if not necessary should be avoided if possible.

But didn't you say to separate it here?

@mtrezza
Copy link
Member

mtrezza commented Oct 15, 2023

Yes, remove the line in the flutter PR #971 (or adapt the CI so that Dart is not tested when running the flutter CI, without using path in the CI), so we can merge the flutter PR first, then rebase this PR and there won't be any issues anymore, right?

@mbfakourii
Copy link
Member Author

Yes, remove the line in the flutter PR #971 (or adapt the CI so that Dart is not tested when running the flutter CI, without using path in the CI), so we can merge the flutter PR first, then rebase this PR and there won't be any issues anymore, right?

Done, please check again

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
packages/dart/CHANGELOG.md Outdated Show resolved Hide resolved
mtrezza
mtrezza previously approved these changes Oct 16, 2023
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.

Looks good, waiting for CI to pass...

@mtrezza mtrezza merged commit 088bfbc into parse-community:master Oct 16, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

Incorrect Dart support policy
2 participants