Set DefaultMetadataReader when ext-exif is not present #841
Conversation
lsmith77
added
the
State: Reviewing
cedricziel
force-pushed
the
metadata-driver
branch
from
017208d to
055ada8
Compare
|
We need to allow for people to manually overwrite this parameter in their own configuration, no? |
robfrawley
added
the
Level: Bug 🐞
|
Yes, sure. I think comparing the set parameter with the bundle default parameter should be sufficient. Fix upcoming. |
|
My thinking exactly. 👍 Also; a test case to ensure this isn't applied when ext-exif is available would be much appreciated. |
cedricziel
force-pushed
the
metadata-driver
branch
from
055ada8 to
f3c460a
Compare
|
That should be it. Now the parameter is only re-set, if it's equal to the default value |
|
Awesome! Should we offer up some kind of warning or log message when the parameter is reset? It's useful behaviour, but this is definitely something which might cause confusion if a user isn't aware of what's going on. |
cedricziel
force-pushed
the
metadata-driver
branch
from
f3c460a to
764a1b6
Compare
|
Agreed. I changed the CompilerPass to emit a log message to the compiler log. |
This fixes the issue where an exif reader is used, but ext-exif is not loaded. Closes: liip#501
cedricziel
force-pushed
the
metadata-driver
branch
from
764a1b6 to
7cba271
Compare
|
I added a basic test. Let me know what you think. :) |
|
I'm pretty happy with this! |
|
@robfrawley you have permissions to merge right? |
|
ah @cedricziel too :) |
lsmith77
removed
the
State: Reviewing
|
Yep, I've just wanted to wait for reviews - thx for participating and the valuable feedback! |
|
@lsmith77 Sure do have merge rights! It's just been a bit since you've been around and many merges have happened with the "new, active maintainers crew"; my review request was just a good way to have you check in and make sure you were comfortable with everything so far. Thanks @cedricziel for the PR! |
This fixes the issue where an exif reader is used, but ext-exif
is not loaded.
Closes: #501