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

fix(fujifilm_ratings.lua): improve the script and make it work again #495

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LucasGGamerM
Copy link

I don't know what changed between 9.2.0 and 9.3.0, but it broke the previous iteration of the script. So I fixed it and improved it a bit.

The main improvement here is that it also checks for JPGs, which I think is necessary. And it works again.

I don't know what changed, but it broke the previous iteration of the script. So I fixed it and improved it a bit.
@wpferguson
Copy link
Member

I don't know what changed between 9.2.0 and 9.3.0

There was an error in the libraries. It was fixed around the time you submitted the PR, so the original code was working again.

The main improvement here is that it also checks for JPGs

The improvement checks all JPGs, not just Fuji jpgs. Since the name of the script is fujifilm_ratings users would get a rude surprise if all their JPGs had their ratings changed by the script.

If you want to write a script that corrects ratings for JPGs that darktable doesn't automatically detect then it should be as a separate script. If you decide to do that, you probably want to check image.exif_maker to make sure that you are only changing ratings for maker images that are not understood by darktable.

@LucasGGamerM
Copy link
Author

Thank you got the feedback, I will see what I can do about it (for some reason my fuji rafs don't have a ratings, while the JPGs do), so maybe another script shall be written

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants