Skip to content

Conversation

@ahocevar
Copy link
Member

@ahocevar ahocevar commented Jun 22, 2016

This is an alternative to #5506. Instead of exposing tile ranges, this pull request makes a tile grid's tile coordinates available by extent and zoom level.

I do not like the name of the new function though. Any suggestions anyone? @tschaub maybe, since you're in a naming mood today it seems?

Fixes #5502.

@ahocevar ahocevar force-pushed the foreachtilecoordinextentandz branch from ca58cc6 to 14cecb7 Compare June 22, 2016 16:05
@ashchurova
Copy link

Do you have an eta? Thanks.

@ahocevar
Copy link
Member Author

This needs to be reviewed by another trusted developer. Since this is not paid work, it might take a while. But it shouldn't be too hard to review this one.

@ahocevar
Copy link
Member Author

@fredj, I cherry-picked your other commit from #5506, so this pull request replaces that one. A review would be much appreciated.

@ahocevar ahocevar changed the title Add function to iterate over tile coordinates in extent and z Make it easier to work with tile ranges in custom source implementations Jun 22, 2016
@fredj
Copy link
Member

fredj commented Jun 23, 2016

LGTM, thanks Andreas

@ahocevar ahocevar merged commit 3726b25 into openlayers:master Jun 23, 2016
@ahocevar ahocevar deleted the foreachtilecoordinextentandz branch June 23, 2016 07:03
@tschaub
Copy link
Member

tschaub commented Jun 23, 2016

For what it's worth, I don't think method names need to repeat the arguments. So, instead of this:

grid.forEachTileCoordInExtentAndZ(extent, z, callback);

my preference would be this:

grid.forEachTileCoord(extent, z, callback);

@ahocevar
Copy link
Member Author

Thanks @tschaub, this was the feedback I was hoping for :-). Please review #5512.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants