Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Remove missing image log #14600

Merged
merged 1 commit into from
May 9, 2019
Merged

Conversation

LukasPaczos
Copy link
Member

Closes #14581.

@LukasPaczos LukasPaczos added the Android Mapbox Maps SDK for Android label May 7, 2019
@LukasPaczos LukasPaczos added this to the release-mojito milestone May 7, 2019
@LukasPaczos LukasPaczos requested a review from tobrun May 7, 2019 09:05
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it is safe to add image ids to the layer that might not be added via style.addImage(id, image).
While it's valid to have a style declaration with missing image resources, It still shows your style setup is not correct/complete and it's something an end user should be aware of. In normal use with using Mapbox default styles, no warning is emitted.

I rather keep this log around but with a lower log level and using a constant formatted string,

@tobrun
Copy link
Member

tobrun commented May 9, 2019

Let me back up my statement above with an actual use-case. I have a custom style in #14582 and the issue ends up being a missing image resource. If pattern images were integrated with missing images as now done in #14605, I would have been able to found the root cause myself instead of needing help from core.

@AlexanderEggers
Copy link
Contributor

@tobrun Wouldn't it make more sense to throw a log once when the map is initialising and not every time the user is moving the camera?

Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with #14610, let's remove the log. Will just use onStyleImageMissing as a debug tool instead.

@LukasPaczos
Copy link
Member Author

LukasPaczos commented May 9, 2019

Will just use onStyleImageMissing as a debug tool instead.

That was my idea. Since having no resources for particular style image can be a legitimate strategy, the callback should be the source of truth.

#14610 doesn't change the log statement targeted here, can we merge this one in this case? misinterpreted

@LukasPaczos LukasPaczos merged commit aa4c89e into master May 9, 2019
@LukasPaczos LukasPaczos deleted the lp-remove-missing-image-log-14581 branch May 9, 2019 09:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(MapBox Android) OnStyleImageMissing throws "none"
3 participants