sooxt98 / google_nav_bar

A modern google style nav bar for flutter.
MIT License
749 stars 114 forks source link

Migrate to null safety #46

Closed Ascenio closed 3 years ago

Ascenio commented 3 years ago
devj3ns commented 3 years ago

Nice work 👍

Will you also migrate the example app?

Because it depends on the library like this: google_nav_bar: path: ../ therefore it won't work now.

Ascenio commented 3 years ago

I could, but unfortunately both badges and line_icons have not migrated yet. There's a PR for badges, and I'm doing one for line_icons.

devj3ns commented 3 years ago

I could, but unfortunately both badges and line_icons have not migrated yet. There's a PR for badges, and I'm doing one for line_icons.

Okay. Then I think the best would be to depend on google_nav_bar in the example like this:

google_nav_bar: ^4.0.2

Then the example should still work.

Or what do you think?

Ascenio commented 3 years ago

I could, but unfortunately both badges and line_icons have not migrated yet. There's a PR for badges, and I'm doing one for line_icons.

Okay. Then I think the best would be to depend on google_nav_bar in the example like this:

google_nav_bar: ^4.0.2

Then the example should still work.

Or what do you think?

I would make it depends on version ^5.0.0 which is going to be the null safe version, however the example can only be migrated once all it's dependencies migrated already. So we can either wait for the icons packages to update or just remove them. I guess the better option is the second because there's no need to have external dependencies in a sample, also it would just take too long.

Ascenio commented 3 years ago

But that would be "drastic" changes to the example, which really comes down to @sooxt98 preferences. Let's see what he thinks.

sooxt98 commented 3 years ago

@Ascenio interesting PR :sparkles: , I think the example should continue works fine with unsound without migration; It just example btw...

Ascenio commented 3 years ago

So @sooxt98, is there anything left to be done before it gets merged? :eyes:

sooxt98 commented 3 years ago

@Ascenio btw i have excluded example folder for publishing, this solve the concern that you mentioned above