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

Merge image generators into a Faker::Image class #1972

Open
connorshea opened this issue May 10, 2020 · 1 comment · May be fixed by #2003
Open

Merge image generators into a Faker::Image class #1972

connorshea opened this issue May 10, 2020 · 1 comment · May be fixed by #2003

Comments

@connorshea
Copy link
Contributor

@connorshea connorshea commented May 10, 2020

Is your feature request related to a problem? Please describe.

I've been using Faker::LoremPixel.image and Faker::Avatar.image for a while in my database seeds and a few months ago the avatar seeder stopped working (I think this is because robohash was moved to being hosted behind Cloudflare, and Cloudflare will sometimes reject requests from Ruby). I wanted to fix this issue by moving the avatar generator to LoremPixel, but it turns out LoremPixel is also having issues and timing out requests pretty often.

I think we should make the codebase simpler and make it easier to switch between these image generators by merging them into one Faker::Image class, with methods like lorem_pixel, robohash, etc. For LoremPixel, it might be a good idea to remove the generator entirely since the site has been so flaky over the years.

If you're adding new objects, please describe how you would use them

Faker::LoremPixel.image would become Faker::Image.lorem_pixel and Faker::Avatar.image would become Faker::Image.robohash. We should also probably move LoremFlickr and Placeholdit into this class.

# Before
Faker::Placeholdit.image(size: '50x50') #=> "https://placehold.it/50x50.png"
Faker::LoremFlickr.image(size: "50x60") #=> "https://loremflickr.com/50/60"
Faker::LoremPixel.image(size: "50x60") #=> "https://lorempixel.com/50/60"
Faker::Avatar.image(size: "50x50") #=> "https://robohash.org/randomstring?size=50x50&set=set1"

# After
Faker::Image.placehold_it(size: '50x50') #=> "https://placehold.it/50x50.png"
Faker::Image.lorem_flickr(size: "50x60") #=> "https://loremflickr.com/50/60"
Faker::Image.lorem_pixel(size: "50x60") #=> "https://lorempixel.com/50/60"
Faker::Image.robohash(size: "50x50") #=> "https://robohash.org/randomstring?size=50x50&set=set1"

I'd also like to add Unsplash as an image source, like faker.js has done.

Describe alternatives you've considered

Making my own generators, I guess? IMO, merging them into one class is the cleanest solution.

Additional context

I made a quick example for an unsplash method (should probably add validations):

class Faker
  class Image < Faker::Base
    class << self
      def unsplash(category: nil, width: 400, height: 400, keyword: nil)
        url = 'https://source.unsplash.com'
        url += "/category/#{category}" unless category.nil?
        url += "/#{width}x#{height}"
        url += "?#{keyword}" unless keyword.nil?
        url
      end
    end
  end
end
@connorshea connorshea linked a pull request that will close this issue May 21, 2020
0 of 4 tasks complete
@lucasqueiroz
Copy link
Member

@lucasqueiroz lucasqueiroz commented May 25, 2020

Suggestion:
Include This person does not exist generator (and the other ones as well - Cats, Horses and Art)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.