Skip to content

Add image formats to Faker\Provider\Image#473

Merged
pimjansen merged 5 commits intoFakerPHP:mainfrom
calebdw:main
May 11, 2022
Merged

Add image formats to Faker\Provider\Image#473
pimjansen merged 5 commits intoFakerPHP:mainfrom
calebdw:main

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

@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.

5 participants