angulardart-community / angular_components

High-quality Material Design components for AngularDart.
https://gallery.angulardart.xyz
22 stars 14 forks source link

Null safety migration #35

Open lejard-h opened 3 years ago

lejard-h commented 3 years ago

Steps to follow

Task

Migrate core

Migrate components (After core migration)

Migrate gallery builders (Last)

How to contribute

GZGavinZhao commented 3 years ago

I've also had the idea of writing unit tests. I think having such a big project with no unit tests is very dangerous. The null-safety migration is a prefect time to do this. I'm thinking about how to organize this...

lejard-h commented 3 years ago

Yes it's a good idea :+1:

I am also wondering if we could directly work on master branch (we don't have automated publish CI yet)

GZGavinZhao commented 3 years ago

@lejard-h Do you think it's better to use a single issue (this one) to track everything and creating an issue for each item on-demand, or to create an issue for every item beforehand and link them to a milestone and this issue?

GZGavinZhao commented 3 years ago

Also, I think we should start this migration after we address #12 and #24.

lejard-h commented 3 years ago

I don't know what's the best option. I just wanted to track everything in one place and easy to access.

For the change detection strategy fix, I think @dukefirehawk already did it in his current PR. It could be extracted in a dedicated PR to merge on master

I could link the issue tracking deprecated stuff here so we can start working on it.

Then we can follow the list of tasks in the order and if the fix already exist in dev branch we can merge it

dukefirehawk commented 3 years ago

@lejard-h I have commented out most deprecated code but hit a wall with angular 7 methods depending on them. Since I could not find any information on their replacement, I have reverted them. I would suggest not looking at them until we figure out its equivalent replacement.

jodinathan commented 3 years ago

I have fixed:
Warnings: doesn't use "ChangeDetectionStrategy.OnPush" #24 Remove uses of deprecated API #12

If I understood correctly, the PR https://github.com/angulardart-community/angular_components/pull/47 should be merged into master and dev (if everything ok) and then we can only focus on making it null safety in the dev branch?

GZGavinZhao commented 3 years ago

@jodinathan Because of my terrible organization, dev branch is currently mostly for @dukefirehawk to work on his PR #30 without us giving him distraction. null-safety is the branch we plan to work on for the migration. I will update the wiki now.

And therefore, #47 will be merged into both the master and null-safety branch.

jodinathan commented 3 years ago

thanks @GZGavinZhao
please let me know when the null-safety branch is ok to migrate

jodinathan commented 2 years ago

@GZGavinZhao @dukefirehawk shouldn't we merge the dev with the master so we have it compatible and a initial null-safety version?

GZGavinZhao commented 2 years ago

@jodinathan In my opinion, not yet. It can build, but there are still runtime errors.

I think now the first priority is to cherry-pick (if merging is possible, then that's even better) @dukefirehawk's work to the null-safety branch. In order to verify it, unit tests should be written for every file that has been changed.

He also wrote a very good CHANGELOG, so we might be able to merge in groups for every list item in the CHANGELOG if the check-per-file method is too tedious.

GZGavinZhao commented 2 years ago

lib/utils fully migrated, though a bit more testing is still needed.

About 60% of them are done by @dukefirehawk on the dev branch. Because it's so hard to cherry-pick (usually one commits contain tens of files), I copied the changes over and marked @dukefirehawk as the author (e.g. 82c9d26e4e0c89cf29a8162b5283fd3583add2c9)

Next stop will be lib/src/utils.