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

Add image formats to Faker\Provider\Image #473

Merged
merged 5 commits into from
May 11, 2022
Merged

Conversation

calebdw
Copy link

@calebdw calebdw commented May 6, 2022

What is the reason for this PR?

The placeholder.com API allows for a few image formats, whereas image() and imageUrl() in Faker\Provider\Image defaulted to png.

  • Slight feature extension

Author's checklist

Summary of changes

Added a static array to Faker\Provider\Image which contains allowable image formats (per Placeholder). Appended $format argument to both Image::image() and Image::imageUrl() which defaults to 'png'. Throw an exception in Image::imageUrl() if an invalid format is received. Wrote unit tests for all the image formats and IllegalArgumentException.

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

src/Faker/Provider/Image.php Outdated Show resolved Hide resolved
src/Faker/Provider/Image.php Outdated Show resolved Hide resolved
src/Faker/Provider/Image.php Outdated Show resolved Hide resolved
@pimjansen
Copy link

Phpcs is failing. Can you review?

@calebdw
Copy link
Author

calebdw commented May 9, 2022

I fixed the above issues, but I am unable to run phpcs myself since I have PHP 8.1 installed on my system.

@pimjansen
Copy link

I fixed the above issues, but I am unable to run phpcs myself since I have PHP 8.1 installed on my system.

Pipeline is green, will do final review tomorrow so we can merge

@pimjansen
Copy link

LGTM

@pimjansen pimjansen merged commit f66a262 into FakerPHP:main May 11, 2022
@jderusse
Copy link

Adding a method in an interface is a BC break.
This PR should have been released in a Major version.

@DvDty
Copy link

DvDty commented Aug 4, 2022

Adding a method in an interface is a BC break. This PR should have been released in a Major version.

Interesting, there is a job in the pipeline that checks for backwards compatibility and it has passed for the current pull request: https://github.com/FakerPHP/Faker/actions/runs/2293913195

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.

None yet

5 participants