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

fix(general bugs): workflow fixes #685

Merged
merged 4 commits into from
Mar 18, 2023

Conversation

amovfx
Copy link
Contributor

@amovfx amovfx commented Feb 26, 2023

-taro nodes connect in order when dropped into the graph
-Taro info asset draw title says 'No Assets' if there are no assets

-taro nodes connect in order when dropped into the graph
-Taro info asset draw title says 'No Assets' if there are no assets
Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

For the Info sidebar, I could have been more specific. The problem I see with this is having a divider with no content below it looks weird. So I was thinking to add text below the divider stating "This node does not have any assets.", so there is content below it.

It looks like code coverage for the network.ts file dropped with this update.

@amovfx
Copy link
Contributor Author

amovfx commented Feb 26, 2023

Thanks for that clarity. Sorry for missing that code coverage. Ill update this asap.

@amovfx
Copy link
Contributor Author

amovfx commented Feb 27, 2023

 network.ts                           |   99.69 |      100 |   98.86 |     100 |

I haven't seen this before, I have 100% line coverage, why would statements and functions be less than 100?

@jamaljsr
Copy link
Owner

Most likely you have an if statement but the else condition is never hit. Open the coverage/lcov-report/index.html for more details on what's not covered.

@amovfx
Copy link
Contributor Author

amovfx commented Feb 27, 2023

Super interesting how a nuanced a language can be. Looks like a sorted array apparently wont sort if it doesn't need to. Pretty cool optimization.

@amovfx amovfx requested a review from jamaljsr February 27, 2023 21:20
Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK 👍

This looks good now. Just one small nitpick.

Renamped Styled.P to Styled.Empty
@amovfx amovfx requested a review from jamaljsr March 18, 2023 19:54
@jamaljsr jamaljsr merged commit e462373 into jamaljsr:feat/add-taro Mar 18, 2023
@amovfx amovfx deleted the fix/general branch March 18, 2023 22:47
jamaljsr pushed a commit that referenced this pull request May 16, 2023
-taro nodes connect in order when dropped into the graph
-Taro info asset draw title says 'No Assets' if there are no assets

---------

Co-authored-by: Andrew Oseen <amovfx@protonmail.com>
jamaljsr pushed a commit that referenced this pull request May 16, 2023
-taro nodes connect in order when dropped into the graph
-Taro info asset draw title says 'No Assets' if there are no assets

---------

Co-authored-by: Andrew Oseen <amovfx@protonmail.com>
jamaljsr pushed a commit that referenced this pull request May 16, 2023
-taro nodes connect in order when dropped into the graph
-Taro info asset draw title says 'No Assets' if there are no assets

---------

Co-authored-by: Andrew Oseen <amovfx@protonmail.com>
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