Skip to content

The writeFeatures method should always return a string #3003

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 5, 2014
Merged

The writeFeatures method should always return a string #3003

merged 1 commit into from
Dec 5, 2014

Conversation

bartvde
Copy link
Member

@bartvde bartvde commented Dec 4, 2014

This PR fixes #2102

All writeFeatures, writeFeature and writeGeometry methods now return a string. The respective methods such as writeFeaturesObject (JSON) and writeFeaturesNode (XML) have been marked api but not stable as yet.

@bartvde bartvde added this to the v3.1.0 milestone Dec 5, 2014
@bartvde
Copy link
Member Author

bartvde commented Dec 5, 2014

This is now ready for review, TIA

* @inheritDoc
* Encode an array of features as a GeoJSON object.
*
* @function
Copy link
Member

Choose a reason for hiding this comment

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

I guess you don't need @function in that case. As I understand it we use @function as an indicator to the docs tool that this is a function. But it's not necessary in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah indeed, good point.

@elemoine
Copy link
Member

elemoine commented Dec 5, 2014

I've just added minor comments. It looks good to me. Thanks for tackling this Bart!

bartvde added a commit that referenced this pull request Dec 5, 2014
The writeFeatures method should always return a string (r=@elemoine)
@bartvde bartvde merged commit 47325cd into openlayers:master Dec 5, 2014
@bartvde bartvde deleted the writefeatures branch December 5, 2014 14:05
@ahocevar
Copy link
Member

ahocevar commented Dec 5, 2014

Post-merge +1 from me. Thanks @bartvde @elemoine

@@ -105,7 +105,7 @@ if ('download' in exportGPXElement) {
clone.getGeometry().transform(projection, 'EPSG:4326');
features.push(clone);
});
var node = new ol.format.GPX().writeFeatures(features);
var node = new ol.format.GPX().writeFeaturesNode(features);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be simplified to use writeFeatures instead of creating a node and then serializing with XMLSerializer below?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point will fix on Monday

Sent from my iPhone

On 6 dec. 2014, at 20:21, Tim Schaub [email protected] wrote:

In examples/gpx.js:

@@ -105,7 +105,7 @@ if ('download' in exportGPXElement) {
clone.getGeometry().transform(projection, 'EPSG:4326');
features.push(clone);
});

  •  var node = new ol.format.GPX().writeFeatures(features);
    
  •  var node = new ol.format.GPX().writeFeaturesNode(features);
    
    Shouldn't this be simplified to use writeFeatures instead of creating a node and then serializing with XMLSerializer below?


Reply to this email directly or view it on GitHub.

@tschaub
Copy link
Member

tschaub commented Dec 6, 2014

Thanks for this work @bartvde.

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.

writeFeatures should return a string for all formats
4 participants