search

dart-archive / http_io

BSD 3-Clause "New" or "Revised" License
15 stars 15 forks source link

Socket implements Stream<Uint8List> #75

Closed tvolkert closed 5 years ago

tvolkert commented 5 years ago

An upcoming change in the Dart SDK will change Socket from implementing Stream<List<int>> to implementing Stream<Uint8List>.

This breaking change in the http_io package corresponds to that breaking change in the SDK. As such, this change also bumps the minimum SDK constraint accordingly.

https://github.com/dart-lang/sdk/issues/36900

tvolkert commented 5 years ago

@kevmoo the failures on this one are expected - I need to land this so I can update https://github.com/dart-lang/sdk/blob/master/DEPS in the SDK breaking change.

tvolkert commented 5 years ago

@sortie I hear you may be a good person to review this (also @zanderso but he's OOO).

I'm curious what the right order of operations is here -- I need to make this change so that I can update the DEPS file in https://dart-review.googlesource.com/c/sdk/+/104524, thus getting CQ tests to pass in that CL. Yet the CQ tests aren't passing in this PR because the Dart SDK change hasn't yet landed... so there seems to be a chicken and egg problem. I assume the process is to land this despite the CQ failures, so that I may update the DEPS file in the main CL.

tvolkert commented 5 years ago

@sortie @whesse @lrhn

tvolkert commented 5 years ago

@sortie yeah there's no forward-compatible way to make the changes, so I'm gonna have to break tests.

I updated the SDK constraint to >2.4.0.

Can you merge this for me since I don't have write access to the repo? Or if you could grant me access to the repo, that works too.

Thanks!

sortie commented 5 years ago

Sounds good to me. 2.5.0-dev.0.0 is probably the most accurate lower bound you can get. Do you want to use that instead of 2.4.0? Let me know and then I can merge this afterwards.

tvolkert commented 5 years ago

Sure - done!

sortie commented 5 years ago

lgtm, I'll land it.