-
-
Notifications
You must be signed in to change notification settings - Fork 923
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
refactor(image)!: randomize defaults #2472
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2472 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 2820 2820
Lines 251738 251751 +13
Branches 1165 1167 +2
=======================================
+ Hits 250607 250634 +27
+ Misses 1102 1088 -14
Partials 29 29
|
Done with the comments! Lunch time for me so please let me know if there's anything left to do on my side so I can work on that later. 🤘🏽 |
We are allowing the zero value to simplify the default value
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.
Looks good to me. Please note that it still wont be merged because it is schedulled for v9.
Thank you so much! a pleasure working with you 🤘🏽 |
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.
Looks good!
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.
While preparing some draft documentation for the breaking change, i found a problem, you can't prevent grey/blurred images when using faker.image.url()
faker.image.url()
picks from faker.image.urlLoremFlickr()
and faker.image.urlPicsumPhotos()
at random. But faker.image.urlPicsumPhotos()
now randomly adds blur and grayscale parameters. If you don't want that you can turn it off using for example
faker.image.urlPicsumPhotos({width:640, height:480, blur:0, grayscale:false})
But faker.image.url
doesn't support the blur and grayscale parameters, so
faker.image.url({width:640, height:480, blur:0, grayscale:false})
still gives you blurry or greyscale pics on the 50% of picsum photos.
Perhaps blur:0 and grayscale:false should be passed to urlPicsumPhotos()
when called by url()
@matthewmayer From what I saw in the test snapshots in the latest commit, that should do the trick, but I don't really know how to test this easily or how you test this (will be good to know if you don't mind sharing it). Let me know if there's something else to fix on my end. |
Per #2521 we want to prepare documentation for breaking changes in advance. Please insert this draft content at
|
@matthewmayer done! |
Ready for review. |
Removes hard-coded defaults and randomizes them. Did this to all methods in the
image
module but didn't touch the deprecated ones.Fixes #2314.