Deprecate LoremPixel

[uzorjchibuzor]

• Add a deprecation warning that LoremPixel is going to be removed in the next release.
• Add Tests for LoremPixel to confirm referral to LoremFlickr

When Faker::LoremPixel.image is called, user is warned that NOTE: Faker::LoremPixel.image is deprecated; use Faker::LoremFlickr.image instead. It will be removed on or after 2022-12.

@stefannibrasil

Corresponding issue #2505

[stefannibrasil]

Hi @uzorjchibuzor thank you for your contribution!

When we deprecate a method, we want to maintain the behavior until the method is removed. So we don't want to comment anything, or remove any code. We just want to add a message warning users to fix the warning message, but the behavior shouldn't change until the method is removed in a future relase.

There is a gem called extend Gem::Deprecate that you can use for this.

What it does is facilitate all of this work for us. Basically you need to extend it, and add a a "@deprecate" method above the method. I recommend checking out the gem's doc for more details.

In the LoremPixel tests, we want to test that the deprecation message is displayed when it's called.

I also realized the LoremFlick test already has a good coverage, at least at a first glance. So I will update the issue to remove this step. We definitely want to test the deprecation message warning for LoremPixel though.

Does all of this make sense?

[Zeragamba]

In this case I think removeal can be done without deprecation as LoremPixel has been non-existant for quite some time now, so even if anyone was calling this generator it'd be returning non-valid urls

[uzorjchibuzor]

Hi @uzorjchibuzor thank you for your contribution!

When we deprecate a method, we want to maintain the behavior until the method is removed. So we don't want to comment anything, or remove any code. We just want to add a message warning users to fix the warning message, but the behavior shouldn't change until the method is removed in a future relase.

There is a gem called extend Gem::Deprecate that you can use for this.

What it does is facilitate all of this work for us. Basically you need to extend it, and add a a "@deprecate" method above the method. I recommend checking out the gem's doc for more details.

In the LoremPixel tests, we want to test that the deprecation message is displayed when it's called.

I also realized the LoremFlick test already has a good coverage, at least at a first glance. So I will update the issue to remove this step. We definitely want to test the deprecation message warning for LoremPixel though.

Does all of this make sense?

It does.

[uzorjchibuzor]

@stefannibrasil the deprecate method requires a month and year, it will not allow a string message. I am using December 2022 for now.

[stefannibrasil]

In this case I think removeal can be done without deprecation as LoremPixel has been non-existant for quite some time now, so even if anyone was calling this generator it'd be returning non-valid urls

I think it's safer to go through the deprecation route. Even if the generator isn't generating valid urls, it might be called somewhere and thus break things when we remove it.

[stefannibrasil]

@uzorjchibuzor thank you for your work on this! I apologize for the late review, I was recovering from a flu 🤧

I left a suggestion. The date looks good! We can leave it as that.

[stefannibrasil]

@uzorjchibuzor thank you for addressing the suggestions! Once the CI is green, this is ready to merge.

[uzorjchibuzor]

@stefannibrasil these test failures are nothing related to LoremPixel, when I checked it was coming from Sports, I will correct the rubocop offense though.

[stefannibrasil]

@stefannibrasil these test failures are nothing related to LoremPixel, when I checked it was coming from Sports, I will correct the rubocop offense though.

Yeah, we can ignore the Sports one for now. There is an open issue to fix that #2608

[stefannibrasil]

Thank you so much for your work on this!

[Zeragamba]

Awesome work :)