Skip to content

Fix getImage(s)Blob() return type #735

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

Merged
merged 1 commit into from
May 9, 2025

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Apr 18, 2025

PR #616 defined the return type of getImageBlob() as nullable, but that is no longer correct because PR #606 changed it to throw an exception instead of returning null.

This PR sets the return type of getImageBlob() back to non-nullable and updates getImagesBlob() to throw an exception when it fails to get the image contents. This makes both methods behave consistently.

@bukka
Copy link
Collaborator

bukka commented Apr 24, 2025

The getImageBlob type change is correct but I'm wondering if we should rather change type back for getImagesBlob to nullable rather than throwing an exception. There might be some code in old version relying on null being returned so we could consider it as a bug.

@HypeMC
Copy link
Contributor Author

HypeMC commented Apr 24, 2025

I'm wondering if we should rather change type back for getImagesBlob to nullable rather than throwing an exception. There might be some code in old version relying on null being returned so we could consider it as a bug.

@bukka I'm fine either way, but the change to getImageBlob was introduced in 3.8.0, so the same argument could be made for anyone updating from 3.7.0. IMO, having consistent behavior that aligns with the docs is the better choice, but I'm happy to change it if you want.

@bukka
Copy link
Collaborator

bukka commented Apr 24, 2025

Good point about the docs. It probably makes sense. I will wait few weeks and if there are no objections, I will merge it.

@bukka
Copy link
Collaborator

bukka commented Apr 24, 2025

Pipeline failure is unrelated - looks like 6.9.2-0 is missing in launchpad

@bukka bukka merged commit b47e91c into Imagick:master May 9, 2025
88 of 91 checks passed
@HypeMC HypeMC deleted the fix-getimagesblob-return-type branch May 9, 2025 20:50
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