-
Notifications
You must be signed in to change notification settings - Fork 0
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
DEV-989 feed: processing jpgs & pngs in digifeed packages #111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reformatting looks fine. There are certainly more opportunities for refactoring, unit testing within ImageRemediate
, and rethinking the relationship and flow of code between ImageRemediate
subclasses and this class, but I don't think we need to worry about that now.
The change I do think we need is some adjustment to headers we're extracting -- in particular we shouldn't copy the image height, width, color depth, etc, as this will all get carried forward when ImageMagick converts to TIFF -- but we do need to make sure to try to get the resolution info from PNGs.
lib/HTFeed/Stage/ImageRemediate.pm
Outdated
|
||
# Build and return a hash of jpg metadata. | ||
sub extract_jpg_metadata { | ||
my $olf = shift; # ref to $self->{oldFields}, a hash of exiftool data |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good to me at this point (assuming that tests in feed_internal pass when running against this branch) Please feel free to do a merge & release when ready.
deleting an old merged branch |
This goes hand in hand with: https://github.com/hathitrust/feed_internal/pull/34