search

shamblett / cbor

A CBOR implementation for Dart
MIT License
36 stars 16 forks source link

NNBD Migration #15

Closed are closed 3 years ago

are commented 3 years ago

After the migration, all existing tests complete successfully. I removed some repetitive getters and replaced them with final. There have been some seemingly unused dev_dependencies in the pubspec.yaml, so I removed them (let me know if they were actually used). This is still blocked by the test and convert dependency conflict, so we will have to wait for that first before merging. To test locally tho, you can add the following to the pubspec:

dependency_override:
  crypto: ^3.0.0-nullsafety.0

This is tracked in https://github.com/dart-lang/web_socket_channel/issues/138.

Let me know your thoughts about this!

shamblett commented 3 years ago

Thanks for this, I think the following dev dependencies are needed for running the unit tests in the browser - build_runner: '^1.7.3'
build_test: '^0.10.12'
build_web_compilers: '^2.8.0' But they probably need updating now anyway.

As for what we do next I've was pinged by the google guys about my xml2json package being identified as popular and important to the ecosystem so I converted this and released it as a null safe version of the package. I don't think we will need to do the same with cbor. I'd rather wait for the conflicts above to be corrected, we may find that if this coincides with Dart 12 being released as stable we can just merge and release from master as normal. If however someone has a need for a null safe cbor before before this I have no problem releasing a null safe package from this branch, we'll just play it by ear with this.

The main thing is the work has been done which is great, thanks again.

are commented 3 years ago

All of the dependencies have migrated! I think this is ready to move forward.

When it comes to those 3 dependencies, I haven't seen any mention of the browser unit tests in the code or travis, so if they are being run manually those packages can always be installed manually. Or we can add them back into the dev dependencies if you prefer.

I would prefer cbor make a prerelease because without it I cannot start with my own package, and I would like to make it ready before stable release.

Let me know your thoughts.

shamblett commented 3 years ago

Looking again at these deps you are right, although the intention was to run the unit tests in the browser we don't actually do this as you have spotted. In theory CBOR can be used in the browser but I don't think I've actually done this. So we can remove them for now and address this as a separate task later.

OK, so do you want me to release a null safety package from your branch? If so I'd rather not merge into master, when I did xml2json I released from its own null safety branch, the plan being to merge this into master when 2.12 hits the streets as stable. So do you want me to create a null safety branch you can merge into or do you have another plan?

are commented 3 years ago

Thanks again for being so responsive and willing to help me with this!

That's a great idea then - you can create a branch and I will edit the PR, then you can merge it and pre-release it at your convenience! Pub supports pre-releasing with special version syntax so it won't affect other users not ready for NNBD yet.

shamblett commented 3 years ago

OK, I've created a branch named nullsafety off master head, please rebase your PR onto this and I'll do the rest.

shamblett commented 3 years ago

OK, cbor 3.3.0-nullsafety.0 published, sorry I was a bit late with this, missed it last week. If you have any othe rproblems or issues with this please update #14 , thanks.