Skip to content

Use writeFeatures instead of writeFeaturesNode in GPX and KML example #3017

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

Merged
merged 1 commit into from
Dec 7, 2014
Merged

Use writeFeatures instead of writeFeaturesNode in GPX and KML example #3017

merged 1 commit into from
Dec 7, 2014

Conversation

bartvde
Copy link
Member

@bartvde bartvde commented Dec 7, 2014

This was pointed out by @tschaub in #3003 (comment) and #3003 (comment)

@elemoine
Copy link
Member

elemoine commented Dec 7, 2014

Looks good to me. Please merge when the build is fixed.

@bartvde
Copy link
Member Author

bartvde commented Dec 7, 2014

right wondering now why the build thinks writeFeatures returns a Node or null ... ;-)

@bartvde
Copy link
Member Author

bartvde commented Dec 7, 2014

ah I see it now, missed an annotation in the source code

@elemoine
Copy link
Member

elemoine commented Dec 7, 2014

I think there is a problem in gpx.js: writeFeatures does not have the correct signature.

@bartvde
Copy link
Member Author

bartvde commented Dec 7, 2014

right I just spotted the same @elemoine will fix

@elemoine
Copy link
Member

elemoine commented Dec 7, 2014

Again, please merge when the build is fixed. Thanks Bart. And thanks Closure Compiler. Believe it or not, I now love CC.

@bartvde
Copy link
Member Author

bartvde commented Dec 7, 2014

it definitely helps, only a shame it was only spotted when using the code in the example ;-)

bartvde added a commit that referenced this pull request Dec 7, 2014
Use writeFeatures instead of writeFeaturesNode in GPX and KML example (r=@elemoine)
@bartvde bartvde merged commit b7d0e06 into openlayers:master Dec 7, 2014
@elemoine
Copy link
Member

elemoine commented Dec 7, 2014

it definitely helps, only a shame it was only spotted when using the code in the example ;-)

We're talking about the return type of an API function that is to be used by applications. So it makes sense that the compiler catches this when compiling the examples. This is also why compiling the examples is actually a good thing.

@bartvde
Copy link
Member Author

bartvde commented Dec 7, 2014

yes I get that, I just wished the normal build step would catch this without having to use all the api code in an example but I'm probably not overseeing whether this is possible at all

Sent from my iPhone

On 7 dec. 2014, at 18:21, Éric Lemoine [email protected] wrote:

it definitely helps, only a shame it was only spotted when using the code in the example ;-)

We're talking about the return type of an API function that is to be used by applications. So it makes sense that the compiler catches this when compiling the examples. This is also why compiling the examples is actually a good thing.


Reply to this email directly or view it on GitHub.

@elemoine
Copy link
Member

elemoine commented Dec 7, 2014

yes I get that, I just wished the normal build step would catch this without having to use all the api code in an example but I'm probably not overseeing whether this is possible at all

Certainly not what you're thinking about but we could have the check target compile the examples code (build/examples/all.combined.js really) at least.

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.

2 participants