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.

Sign up for GitHub

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

Jump to bottom

Fix 'video.tools.drawing.color_gradient' #1467

Merged
merged 11 commits into from
Jan 19, 2021
Merged

Fix 'video.tools.drawing.color_gradient' #1467

merged 11 commits into from
Jan 19, 2021

Conversation

mondeja
Copy link
Collaborator

@mondeja mondeja commented Jan 18, 2021 •
edited
Loading

Closes #1256

@mondeja mondeja added video Related to VideoClip and related classes, or handling of video in general. bug-fix For PRs and issues solving bugs. labels Jan 18, 2021
@coveralls
Copy link

coveralls commented Jan 18, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 65.883% when pulling 00884e3 on mondeja:fix-1256 into 3ebede0 on Zulko:master.

@tburrows13
Copy link
Collaborator

I think that this still needs a bigger rework? The issue is that circle() doesn't provide p2 or vector, and I think that the code will still fail in this case.

@mondeja mondeja requested a review from tburrows13 January 19, 2021 01:37
@mondeja mondeja requested a review from tburrows13 January 19, 2021 01:41
@mondeja mondeja changed the title Fix arguments inconsistency in 'video.tools.drawing.color_gradient' Fix 'video.tools.drawing.color_gradient' Jan 19, 2021
@mondeja mondeja requested review from tburrows13 and removed request for tburrows13 January 19, 2021 01:48
YannickAaron
YannickAaron approved these changes Jan 19, 2021
View reviewed changes
Copy link

@YannickAaron YannickAaron left a comment

Choose a reason for hiding this comment

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

fix works

@mondeja
Copy link
Collaborator Author

mondeja commented Jan 19, 2021 •
edited
Loading

I've done manual testing with all possible combinations and I'm sure that now works and is more efficient, so I'm merging... In the future would be nice to have some proper tests.

@mondeja mondeja merged commit 7183eaa into Zulko:master Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YannickAaron YannickAaron YannickAaron approved these changes

@tburrows13 tburrows13 Awaiting requested review from tburrows13

Assignees
No one assigned
Labels
bug-fix For PRs and issues solving bugs. video Related to VideoClip and related classes, or handling of video in general.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

“The End” effect example scripts get fault -- TypeError: 'NoneType' object is not subscriptable
4 participants
@mondeja @coveralls @tburrows13 @YannickAaron