avioli / uni_links

Flutter plugin for accepting incoming links.
BSD 2-Clause "Simplified" License
564 stars 317 forks source link

Null Safety Migration #103

Closed Arcturus5404 closed 3 years ago

Arcturus5404 commented 3 years ago

Is there any change this library getting null safety support?

orevial commented 3 years ago

This library is actually already null safe, I think the only thing for Flutter/Dart to recognize it is to migrate to Flutter 2, which I've proposed in PR #104

Arcturus5404 commented 3 years ago

Awesome!

miDeb commented 3 years ago

Well, the only missing thing is to push a new version to pub.dev. @avioli could you take care of that?

higorlapacw commented 3 years ago

Any news on the push?

objectivecosta commented 3 years ago

Bump!

davidpanic commented 3 years ago

hello???

objectivecosta commented 3 years ago

I don't know if @avioli is alright or just took some time off, either way, I made a simplified version for personal usage while the lib is not updated!

https://github.com/objectivecosta/uni_links_simplified

Hope everything is all right!

Arcturus5404 commented 3 years ago

@objectivecosta Is this a nullsafe variant of this lib?

Arcturus5404 commented 3 years ago

@orevial maybe you should push a version to pub.dev too, call it flutter_uni_links

BringTheChill commented 3 years ago

@objectivecosta I agree with @Arcturus5404 . Can you do this?

objectivecosta commented 3 years ago

@Arcturus5404 @BringTheChill pushing another version of the lib is not the solution. I made a simplified repo which you can use to develop ATM, just add it as a git dependency on the pubspec.yaml. Pushing to pub.dev just so a few more lines on the pubspec could be saved is not a good solution.

Let's just wait for @avioli.

BringTheChill commented 3 years ago

@objectivecosta thank you anyway for your effort with this. you helped me a lot!

objectivecosta commented 3 years ago

If anyone else need temporary help running this, just update your pubspec.yaml

# uni_links: ^0.4.0
  uni_links:
    git:
      url: git@github.com:objectivecosta/uni_links_simplified.git

DISCLAIMER: You'll be running the simplified version I put in https://github.com/objectivecosta/uni_links_simplified

orevial commented 3 years ago

I agree, forking the plugin to pub.dev is not a good solution as it will imply that the two packages will forever live side-by-side and efforts to maintain one of the other are duplicated which is not good IMHO.

I'd rather wait for @avioli to hopefully come back and in the mean time we can all depend on a Git branch for a null-safe version of the plugin 😉

Arcturus5404 commented 3 years ago

@objectivecosta Can you explain what the simplified means? What did you remove for instance?

Arcturus5404 commented 3 years ago

If you changed too much, a better solution for now would be:

uni_links: ^0.4.0
  uni_links:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration

Note: I havent tested it yet

objectivecosta commented 3 years ago

@Arcturus5404 uni_links depends on pub.dev's uni_links_web, which is not published yet. I just put it all in one package.

cbenhagen commented 3 years ago

You need to override all three packages:

  uni_links:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links
  uni_links_platform_interface:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links_platform_interface
  uni_links_web:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links_web
orevial commented 3 years ago

You need to override all three packages:

  uni_links:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links
  uni_links_platform_interface:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links_platform_interface
  uni_links_web:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration
      path: uni_links_web

Not really, it won't work as I kept local versions dependencies (the author needs to update the platform interface first and then upgrade uni_links_weband uni_links dependecy version to uni_links_platform_interface). With your solution you will probably get an error such as this one:


Because every version of uni_links_web from git depends on uni_links_platform_interface from hosted and YOUR-APP depends on uni_links_platform_interface from git, uni_links_web from git is forbidden.

In the meantime I created another branch with the same code but where uni-links-* dependencies are pointing to the same Git repository so each dependecy can be resolved. To use it:

uni_links:
    git:
      url: git@github.com:orevial/uni_links
      ref: null-safety-migration-local-git-repo
      path: uni_links

Note that this way you don't need to depend on inner packages (web/platform_interface)

avioli commented 3 years ago

Done. Thanks @orevial