Conversation
skoudoro
left a comment
There was a problem hiding this comment.
Hi @ManishReddyR,
Thank you for this PR. Overall, it looks good.
- Can you add some unit tests ?
- There are some formatting issue( e.g: https://github.com/fury-gl/fury/actions/runs/12846827886/job/35825810717?pr=959#step:4:179). Can you check that and fix all of them. Thank you !
|
Also, it would be great to have a screenshot (with and without orientation and color) |
|
Hi @ManishReddyR, PR looks good! |
|
Also, comment the snapshot part for now until we find a fix for this function |
TEST: Adding uni tests for square actor
skoudoro
left a comment
There was a problem hiding this comment.
Hi @ManishReddyR,
See my review below. Please, pay attention to details if you want your PR to be merged fast.
Thank you
|
Hi @skoudoro Thank you |
|
in this case, it is ok. But usually, better to split in multiple PR |
|
Hello @skoudoro Thank you |
skoudoro
left a comment
There was a problem hiding this comment.
Thanks for the update, see below
skoudoro
left a comment
There was a problem hiding this comment.
Thanks! all good, merging !


Tried actor for cylinder