Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

add random GET argument for imageUrl faker #372

Closed
wants to merge 4 commits into from

Conversation

Nicals
Copy link
Contributor

@Nicals Nicals commented Jul 18, 2014

When using multiple imageUrl with same configuration, only one image is loaded from lorempixel since the generated url are the same.

This pull requests adds an argument to tell Faker to add a random GET parameter to the generated url in order to have different images from lorempixel.

@ronanguilloux
Copy link
Contributor

Some Travis-CI trouble may be involved here :

Travis' output mentioned some temporary network-related error:

php5.4:

[Composer\Downloader\TransportException]                                     
  The "https://getcomposer.org/version" file could not be downloaded: php_net  
  work_getaddresses: getaddrinfo failed: Temporary failure in name resolution  
  failed to open stream: php_network_getaddresses: getaddrinfo failed: Tempor  
  ary failure in name resolution

hhvm:

[Composer\Downloader\TransportException]                                     
  The "https://getcomposer.org/version" file could not be downloaded: Failed   
  to open https://getcomposer.org/version (Couldn't resolve host 'getcomposer  
  .org') 

Maybe @Nicals shall try a new test commit to force Travis-ci to check this PR again ?

@fzaninotto
Copy link
Owner

I don't understand. LoremPixel returns always a different image, even when called with the same URI. Can you provide a failing test to demonstrate your problem?

@Anahkiasen
Copy link

No it won't, not during the same pageload. If you hit 5 times the same URL in a single page it'll display 5 times the same, and if you refresh 5 times a new one.

@Nicals
Copy link
Contributor Author

Nicals commented Aug 15, 2014

Ok, forgot about this pull request. It now passes as Travis network don't suck anymore.

@fzaninotto
Copy link
Owner

Now I understand, but why only 5? It doesn't make sense to me.

@Nicals
Copy link
Contributor Author

Nicals commented Aug 23, 2014

I don't understand the problem. Only 5 what ?

All is done in this patch is add a 5 digits random GET argument to the image url in order to have different images within the same web page instead of multiple time the same one.

@fzaninotto
Copy link
Owner

My bad, I'm mixing up randomNumber and numberBetween.

I'm ok with the patch, could you just modify the README accordingly? Does it make sense to make the $randomize parameter default to true?

@Nicals
Copy link
Contributor Author

Nicals commented Aug 26, 2014

I think it doesn't make sense to make the $randomize true. Different images mean more client resources loading time. On large page, I would prefer a small number of images. So the default configuration to false is IMHO preferable.

I also think it's better to set it to False so that existing Faked program would have the same behaviour when the developer updates the Faker lib. Not a big deal on such an option, I know, but I think it's preferable. Adding feature is a good think, but it shouldn't alter existing code.

@fzaninotto
Copy link
Owner

I don't agree. Users expect the provider to return real random data, the fact that the same image appears several times on a given page is seen as a bug (even by me).

@fzaninotto
Copy link
Owner

Superseded by #494

@fzaninotto fzaninotto closed this Jan 4, 2015
@fzaninotto
Copy link
Owner

Merged in the other PR, thanks for your contribution!

@Nicals Nicals deleted the lorempixel_randomize branch January 8, 2015 05:53
@Nicals Nicals restored the lorempixel_randomize branch January 8, 2015 05:54
@Nicals Nicals deleted the lorempixel_randomize branch January 8, 2015 05:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants