search

jmathai / elodie

An EXIF-based photo assistant, organizer and workflow automation tool.
https://bit.ly/introducing-elodie
Apache License 2.0
1.25k stars 137 forks source link

Resolved two issues: added support for MKV files and clarified "error" printed output #461

Open D3Zyre opened 9 months ago

D3Zyre commented 9 months ago

resolved: #457 resolved: #460

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

D3Zyre commented 9 months ago

The fail on ci/circleci is due to the printed output being changed in result.py. This is expected behavior for this PR (Issue #460) the solution to #460 was to clarify in the printed output when files are not imported due to being duplicates of already imported files, instead of referring to them as "errors"

D3Zyre commented 9 months ago

I am working on updating the tests for my code, I'll need to create another PR when it's done.

I have noticed that my code does fail 4 tests, even after updating the tests. so I will figure that out now

coveralls commented 9 months ago

Coverage Status

coverage: 90.884% (+0.3%) from 90.61% when pulling 6e946d6287c19bfb1b578a7ea4583cbcdc54097c on D3Zyre:main into 76ad82303fcbf0e4ab421dbfbb39e4a4cc76aecc on jmathai:master.

D3Zyre commented 9 months ago

oh I don't need to make a new PR apparently. cool.

Well as you can see I've updated the tests to the new formatting and fixed the minor issue that had caused 4 of them to fail

D3Zyre commented 9 months ago

On this note:

We should really add some test cases to test the functionality of trying to import a file that has already been imported, to see if the duplicate count goes up

D3Zyre commented 9 months ago

the test that has now failed (for whatever reason) has nothing to do with any code I have changed. Odd

D3Zyre commented 9 months ago

I changed nothing and now the test passes. It seems that the failed test was a HTTPS call to that geolocation server. Annoying

D3Zyre commented 9 months ago

Another note. Is there any documentation that needs to be updated with this "UI" change?