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.

Sign up for GitHub

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

Jump to bottom

TerrainExtension (3/3) #7608

Merged
merged 19 commits into from
Feb 13, 2023
Merged

TerrainExtension (3/3) #7608

merged 19 commits into from
Feb 13, 2023

Conversation

Pessimistress
Copy link
Collaborator

For #7195

Change List

  • Unit tests
  • Render tests
  • Increased render test timeout to 10s

@Pessimistress Pessimistress force-pushed the x/terrain-3 branch 3 times, most recently from 8e83377 to 3628f75 Compare January 30, 2023 20:07
@Pessimistress
Copy link
Collaborator Author

Raster tiles are checked into test/data following what we do with the MVTLayer render tests. CI seems to have really bad network bandwidth and is unable to load tiles from a remote server before timeout. We may be able to host them in deck.gl-data and pull them in with submodule or something, but I need to look into that.

@Pessimistress Pessimistress force-pushed the x/terrain-2 branch 2 times, most recently from 7cc72aa to 5e15705 Compare January 31, 2023 00:14
@alasarr alasarr added this to the 8.9 milestone Feb 2, 2023
@Pessimistress Pessimistress force-pushed the x/terrain-3 branch 3 times, most recently from 682c953 to e096400 Compare February 10, 2023 22:30
@Pessimistress Pessimistress force-pushed the x/terrain-3 branch 2 times, most recently from c49c298 to fc48404 Compare February 10, 2023 22:35
@ibgreen
Copy link
Collaborator

ibgreen commented Feb 13, 2023

Raster tiles are checked into test/data following what we do with the MVTLayer render tests. CI seems to have really bad network bandwidth and is unable to load tiles from a remote server before timeout. We may be able to host them in deck.gl-data and pull them in with submodule or something, but I need to look into that.

My understanding was that openstreetmap data has a contagious license and we should try hard to avoid having any OSM data in the same repos as other data or code.

So yes, It might be worth setting up a small deck.gl-data-osm repo and use git submodule in yarn bootstrap.

ibgreen
ibgreen approved these changes Feb 13, 2023
View reviewed changes
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

just cleanup and test cases...

@ibgreen
Copy link
Collaborator

ibgreen commented Feb 13, 2023

Increased render test timeout to 10s

I have noticed that the render tests already do take a lot of time. I'd be curious to understand what drives that.

Base automatically changed from x/terrain-2 to master February 13, 2023 20:52
@Pessimistress Pessimistress merged commit 5ca046e into master Feb 13, 2023
@Pessimistress Pessimistress deleted the x/terrain-3 branch February 13, 2023 21:23
@coveralls
Copy link

Coverage Status

Coverage: 90.264% (+1.4%) from 88.855% when pulling a8607ae on x/terrain-3 into dbfb31c on master.

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

@ibgreen ibgreen ibgreen approved these changes

@felixpalmer felixpalmer Awaiting requested review from felixpalmer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
8.9
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Pessimistress @ibgreen @coveralls @alasarr