Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null safety migration #35

Open
9 of 54 tasks
lejard-h opened this issue Oct 23, 2021 · 12 comments
Open
9 of 54 tasks

Null safety migration #35

lejard-h opened this issue Oct 23, 2021 · 12 comments
Labels
help wanted Extra attention is needed null safety Related to null safety migration

Comments

@lejard-h
Copy link
Collaborator

lejard-h commented Oct 23, 2021

Steps to follow

Task

Migrate core

  • Typed OpaqueToken The argument type 'Object' can't be assigned to the parameter type xxx #36
  • lib/utils/ @GZGavinZhao
  • lib/theme/
  • lib/stop_propagation/stop_propagation.dart
  • lib/src/laminate/
  • lib/src/model/
  • lib/src/utils/ @GZGavinZhao
  • lib/model/
  • lib/interfaces/has_disabled.dart
  • lib/laminate/
  • lib/framework_stabilizers/
  • lib/forms/error_renderer.dart
  • lib/focus/
  • lib/content/
  • lib/annotations/
  • lib/auto_dismiss/
  • lib/dynamic_component/dynamic_component.dart
  • lib/button_decorator/button_decorator.dart

Migrate components (After core migration)

  • lib/glyph
  • lib/material_icon/
  • lib/material_spinner/
  • lib/material_popup/
  • lib/material_dialog/
  • lib/highlighted_text/
  • lib/material_button/
  • lib/material_card/
  • lib/material_checkbox/
  • lib/material_chips/
  • lib/material_datepicker/
  • lib/material_expansionpanel/
  • lib/material_input/
  • lib/material_list/
  • lib/material_menu/
  • lib/material_progress/
  • lib/material_radio/
  • lib/material_ripple/
  • lib/material_select/
  • lib/material_slider/
  • lib/material_stepper/
  • lib/material_tab
  • lib/material_toggle
  • lib/material_tooltip
  • lib/material_tree
  • lib/material_yes_no_buttons
  • lib/reorder_list
  • lib/scorecard
  • lib/simple_html
  • lib/app_layout/

Migrate gallery builders (Last)

  • angular_gallery
  • angular_gallery_section

How to contribute

  • Create an issue based on the task you want to do.
  • 1 Task = 1 PR.
  • Submit PR on dev branch
  • Avoid change not related to null safety.
  • Avoid dart migrate automated migration
  • Migrate existing example at the same time if any
  • Test your component on the Gallery cd examples/angular_components_example && dart run build_runner serve web
@lejard-h lejard-h added help wanted Extra attention is needed null safety Related to null safety migration labels Oct 23, 2021
@GZGavinZhao
Copy link

GZGavinZhao commented Oct 23, 2021

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
Copy link
Collaborator Author

Yes it's a good idea 👍

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

@GZGavinZhao GZGavinZhao added this to the Null Safety milestone Oct 24, 2021
@GZGavinZhao
Copy link

@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
Copy link

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

@lejard-h
Copy link
Collaborator Author

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
Copy link
Collaborator

dukefirehawk commented Oct 25, 2021

@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.

@GZGavinZhao GZGavinZhao removed this from the Null Safety milestone Oct 25, 2021
@GZGavinZhao GZGavinZhao pinned this issue Oct 27, 2021
@jodinathan
Copy link
Collaborator

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

If I understood correctly, the PR #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
Copy link

@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
Copy link
Collaborator

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

@jodinathan
Copy link
Collaborator

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

@GZGavinZhao
Copy link

GZGavinZhao commented Nov 12, 2021

@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
Copy link

GZGavinZhao commented Jan 18, 2022

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. 82c9d26)

Next stop will be lib/src/utils.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed null safety Related to null safety migration
Projects
None yet
Development

No branches or pull requests

4 participants