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

Exif from PNGs with eXIf chunk #21

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Conversation

paxvista
Copy link
Contributor

No description provided.

@lovell
Copy link
Collaborator

lovell commented Oct 12, 2022

Hi Bob, thank you for the PR, this should fix #19

Did you see a possible alternative approach from @rexxars in commit sanity-io@6ee2ee2 ? If nothing else it might be good to include the test case here also.

@paxvista
Copy link
Contributor Author

The approach from @rexxars is definitely more elegant, but creates a new buffer, which was something I was trying to avoid. Are you suggesting I include his test in my pull request?

Copy link
Collaborator

@lovell lovell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I agree with the use of offsets rather than copy/concat. I'll leave this here for a few days for any further feedback from other maintainers/contributors before merging. Thank you!

@squaredx
Copy link

squaredx commented Dec 1, 2022

Hi all! Is there any status update on when this will be merged into main?

@lovell
Copy link
Collaborator

lovell commented Dec 6, 2022

Sure, there's been no other feedback, let's merge.

@lovell lovell merged commit 0131110 into devongovett:master Dec 6, 2022
@squaredx
Copy link

squaredx commented Dec 6, 2022

@lovell thank you so much! 🙌

@ToPeas
Copy link

ToPeas commented Dec 9, 2022

how long will the publish new release? thanks. i really need this feature that parse png exif.

@lovell lovell mentioned this pull request Dec 13, 2022
@lovell
Copy link
Collaborator

lovell commented Dec 22, 2022

v1.1.0 now available via npm

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.

4 participants