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

Use all url templates provided by terrain config file #3038

Merged
merged 1 commit into from
Sep 22, 2015

Conversation

oterral
Copy link

@oterral oterral commented Sep 18, 2015

Ready to review.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 18, 2015

Thanks for the contribution, @oterral.

I don't believe we have a Contributor's License Agreement (CLA) for you or your organization. Can you send one in? See https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CONTRIBUTING.md

Also, check out the pull request guidelines in that doc. For example, I imagine we will want to include a unit test and update to CHANGES.md for this change.

@oterral
Copy link
Author

oterral commented Sep 18, 2015

Yep I can try.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 18, 2015

Great, thanks @oterral. If there's any questions about the CLA, feel free to contact me directly.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 18, 2015

We have a CLA now.

@kring can you review this?

@oterral
Copy link
Author

oterral commented Sep 18, 2015

And a test :)

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 18, 2015

Thanks @oterral.

@kring this looks good to me. Can you confirm and merge?

}).then(function() {
spyOn(loadWithXhr, 'load');
provider.requestTileGeometry(0, 0, 0);
expect(loadWithXhr.load.calls.mostRecent().args[0]).toContain('foo0.ch');
Copy link
Member

Choose a reason for hiding this comment

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

This test is currently failing because the json specifies .com and the test expects .ch. Otherwise this looks good and I'll merge it as soon as the test is fixed.

Copy link
Author

Choose a reason for hiding this comment

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

I fix

@oterral
Copy link
Author

oterral commented Sep 22, 2015

@kring I've fixed the test.

@kring
Copy link
Member

kring commented Sep 22, 2015

Thanks @oterral!

kring added a commit that referenced this pull request Sep 22, 2015
Use all url templates provided by terrain config file
@kring kring merged commit bcbdc7a into CesiumGS:master Sep 22, 2015
@oterral
Copy link
Author

oterral commented Sep 22, 2015

Thx for the review!!!

@oterral oterral deleted the multiurl branch October 31, 2017 13:26
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.

None yet

3 participants