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

Fix the thumbnail() method return type #2463

Merged
merged 3 commits into from
Sep 15, 2021
Merged

Conversation

titouanmathis
Copy link
Contributor

Issue

The returned value of the Post::thumbnail() method is considered to be \Timber\Timber\Image as we already are in the Timber namespace and Timber\Image is missing a leading \. This is making tools such as PHPStan to fail.

Solution

The solution is to use the same type as the Post::get_thumbnail() method, which is directly using the Image class from the current namespace.

Impact

It will improve compatibility with static analysis tools such as PHPStan.

Testing

Nothing to update (I think).

With `Timber\Image` the returned value is considered to be `\Timber\Timber\Image` as we already are in the `Timber` namespace, making tools such as PHPStan to fail.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 92.868% when pulling be2c78b on titouanmathis:master into 384f130 on timber:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 92.868% when pulling be2c78b on titouanmathis:master into 384f130 on timber:master.

@coveralls
Copy link

coveralls commented Jul 1, 2021

Coverage Status

Coverage remained the same at 92.868% when pulling 8ff4a54 on titouanmathis:master into 8598ce5 on timber:master.

@jarednova jarednova added the 1.x label Sep 15, 2021
@jarednova
Copy link
Member

Thanks @titouanmathis; sorry this has taken me a while — but this looks good and will merge in!

@jarednova jarednova merged commit 81e4774 into timber:master Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants