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

Small fix to resolve unicode error #901

Closed

Conversation

TheShadow29
Copy link

@TheShadow29 TheShadow29 commented Jan 15, 2019

Its a small fix for unicode errors. Solves #140 and maybe #207. To the best of my understanding (https://docs.python.org/3/howto/unicode.html) this should not cause any issues. However, I am not sure what a good test would be.

  • If this is a bugfix, I have provided code that clearly demonstrates the problem and that works when used with this PR
  • I have added a test to the test suite, if necessary
  • I have properly documented new or changed features in the documention, or the docstrings
  • I have properly documented unusual changes to the code in the comments around it
  • I have made note of any breaking/backwards incompatible changes

@Overdrivr
Copy link
Collaborator

Hi @TheShadow29, thanks for your contribution. Could you please:

  • rebase your branch against master as there as been some changes on this piece of code
  • write a new test case for ffmpeg_parse_infos, by simply parsing the faulty video you've mentioned here?

Thanks

@Overdrivr Overdrivr added the tests-needed PRs/code submissions which need test cases added to them. label Mar 27, 2019
@TheShadow29
Copy link
Author

Sure. I will try to do it sometime on the weekend

@daniestrella1
Copy link

Is this fix added on release 1.0.0?

@tburrows13
Copy link
Collaborator

Hi, it looks like this section of code was changed in 350571a so this PR can be closed as it is no longer relevant. The new fix is in #959 so feel free to try that change out and report back on it.

@tburrows13 tburrows13 closed this Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-needed PRs/code submissions which need test cases added to them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants