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

feat: optimize the table node parse steps and add logs #875

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

LucasXu0
Copy link
Collaborator

@LucasXu0 LucasXu0 commented Sep 4, 2024

Remove the assertion and forced unwrapping, make the code more production-friendly, and add more logging.

@LucasXu0
Copy link
Collaborator Author

LucasXu0 commented Sep 4, 2024

@zoli FYI, you may be interested in this PR. :)

Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 65.45455% with 19 lines in your changes missing coverage. Please review.

Project coverage is 72.48%. Comparing base (da0a56c) to head (84d22d7).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...t/table_block_component/table_block_component.dart 64.00% 9 Missing ⚠️
...le_block_component/table_cell_block_component.dart 0.00% 5 Missing ⚠️
...ck_component/table_block_component/table_node.dart 80.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #875      +/-   ##
==========================================
- Coverage   72.56%   72.48%   -0.09%     
==========================================
  Files         316      316              
  Lines       14732    14757      +25     
==========================================
+ Hits        10690    10696       +6     
- Misses       4042     4061      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LucasXu0 LucasXu0 merged commit 44989c5 into AppFlowy-IO:main Sep 4, 2024
8 of 12 checks passed

for (var i = 0; i < colsCount; i++) {
if (node.children.length != colsLen * rowsLen) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you just call validate on the node (maybe with some customization on the validate function)?

@zoli
Copy link
Contributor

zoli commented Sep 4, 2024

@zoli FYI, you may be interested in this PR. :)

Thanks. I saw an issue regarding the table node the root of that problem hasn't been found yet. right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants