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

Bump min SDK to Dart 2.18.0 / Flutter 3.3.0 & fix CI #2176

Merged
merged 16 commits into from
Apr 18, 2024

Conversation

kuhnroyal
Copy link
Member

@kuhnroyal kuhnroyal commented Apr 3, 2024

  • Bump min SDK to Dart 2.18.0 / Flutter 3.3.0 where we can use a newer melos version (3.4.0) that allows simplified usage and supports pubspec_overrides.yaml files
  • revert workflow event back to pull_request & create coverage comment in a separate workflow
  • remove unnecessary exclusive parameter
  • adapt download test to use httpbun instead of downloading artifacts from Github

New Pull Request Checklist

  • I have read the Documentation
  • I have searched for a similar pull request in the project and found none
  • I have updated this branch with the latest main branch to avoid conflicts (via merge from master or rebase)
  • I have added the required tests to prove the fix/feature I'm adding
  • I have updated the documentation (if necessary)
  • I have run the tests without failures
  • I have updated the CHANGELOG.md in the corresponding package

Additional context and info (if any)

@kuhnroyal kuhnroyal added the infra label Apr 3, 2024
@kuhnroyal
Copy link
Member Author

Well this doesn't work

@kuhnroyal
Copy link
Member Author

Looks like something is cached

@kuhnroyal
Copy link
Member Author

Ok, so since we switched to pull_request_target a lot of the repository content is actually used from main. Probably when we use path or git dependencies.

@AlexV525
Copy link
Member

AlexV525 commented Apr 4, 2024

Ok, so since we switched to pull_request_target a lot of the repository content is actually used from main. Probably when we use path or git dependencies.

Ah that sounds fair. Maybe GITHUB.ref would help?

@kuhnroyal kuhnroyal changed the title Remove exclusive: false parameter in test case CI fixes Apr 4, 2024
@kuhnroyal
Copy link
Member Author

kuhnroyal commented Apr 4, 2024

So after reverting to pull_request we still have 2 issues.

  1. The download test is taking too long (longer than 2 minutes)
  2. The melos version on min SDK (or the Dart version itself) doesn't seem to support the pubspec overrides

@kuhnroyal kuhnroyal changed the title CI fixes Bump min SDK to Dart 2.18.0 / Flutter 3.3.0 & fix CI Apr 4, 2024
@kuhnroyal
Copy link
Member Author

Dart 2.18.0 was released on Aug 30, 2022 - so I think this is fine, we should probably publish this as a minor version.

@kuhnroyal kuhnroyal marked this pull request as ready for review April 4, 2024 11:15
@kuhnroyal kuhnroyal requested a review from a team as a code owner April 4, 2024 11:15
@kuhnroyal
Copy link
Member Author

The build still shows as failure but this is due to the old pull_request_target usage.

@kuhnroyal kuhnroyal self-assigned this Apr 4, 2024
@kuhnroyal
Copy link
Member Author

@AlexV525 I think this is good to go, the coverage comment should start to work again once the new workflow is on the main branch. Hopefully it also has correct coverage values after this. This was likely caused by the same problem.

@AlexV525
Copy link
Member

AlexV525 commented Apr 5, 2024

I'm on a holiday and will be AFK for 3 days. I'm considering a couple things:

  1. I'd like to fix 🔥 Do not detaches sockets / terminates client stream #2173 first so we can get as much fixes as we could before we bump SDK requirements.
  2. I'd also like if we split the exclusive fix as a single request so we can easily manage the SDK upgrade request.
  3. We might also stated somewhere that we can at least support SDKs that are released within 1 year in order to provide a solid service.

@kuhnroyal
Copy link
Member Author

Have a nice holiday!

I understand your concerns but as it stands now, the min Dart SDK 2.15.0 does not support pubspec_overrides.yaml.
So all the testing setup we did recently does not actually work for the min SDK since we removed the path dependencies to the test project.

All changes in this PR are interwoven and atomically won't solve the problems.
If we change the exclusive problem, there would still be wrong tests pointing to the main branch in the next PR. It might show a success while the actual code does not work, just how the exclusive problem happened in the first place.

Dart 2.15 ist almost 2.5 years old, we need to get rid of it so we can move forward with the limited time we all have to spare for OS projects. If someone is still using such an old version and having actual problems due to specific bugs, then they can afford to pay someone else to solve the problem in a fork or donate to dio and request a custom handmade release.

@AlexV525
Copy link
Member

AlexV525 commented Apr 5, 2024

Yes I agree with all changes, those concerns are something I think we might do when applying changes. As our regular principle about the maintaining, we can provide a great guide or message to allow users to have a good chance to stand along with our opinions.

# Conflicts:
#	dio_test/lib/src/test/download_tests.dart
@AlexV525
Copy link
Member

I've added the compatibility policy doc to indicate our general support and exceptions.

@kuhnroyal
Copy link
Member Author

Nice, I would go with ~1.5 years instead of 2 years.
We have Dart 3.0.0 on the 6.0.0 branch which is less than 1 year ago. We might be able to go back down to 2.19.0 and then we could release this around mid 2024.

@AlexV525
Copy link
Member

Nice, I would go with ~1.5 years instead of 2 years.

Yeah it's a general idea, so it depends, which means you could always do so if you have a proper reason.

We have Dart 3.0.0 on the 6.0.0 branch which is less than 1 year ago. We might be able to go back down to 2.19.0 and then we could release this around mid 2024.

Sounds good

@AlexV525
Copy link
Member

@ueman Do you have any input about this request and relevant policies?

@AlexV525 AlexV525 merged commit 6a77d56 into cfug:main Apr 18, 2024
3 of 6 checks passed
@kuhnroyal kuhnroyal deleted the fix-ci branch April 18, 2024 15:09
@AlexV525 AlexV525 mentioned this pull request Aug 8, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants