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

Remove ol.format.BinaryFeature #3516

Merged
merged 4 commits into from Apr 19, 2015
Merged

Remove ol.format.BinaryFeature #3516

merged 4 commits into from Apr 19, 2015

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented Apr 7, 2015

I'm excited to have parsers for binary encodings, but this code is neither tested nor used currently. And the addition of ArrayBuffer to the ol.format.Feature#readFeature methods is misleading (see also #3515). Since this will forever be in our history, when we do have a specific implementation of a feature format for binary encodings, the author can decide if there is utility in this code.

There's more that can be removed after #3481 is in (specifically ol.format.FormatType.BINARY and related code in the feature loader).

@elemoine
Copy link
Member

elemoine commented Apr 7, 2015

Ok for me, but I think it would be nice that the removal of the binary format be done with one commit or consecutive commits, to make it easier for someone who wants to find back the old binary format code.

@marcjansen
Copy link
Member

I am too slow apparently... see 7a8fa2c of #3517 (WIP)

Plus one for the removal, though.

@fredj
Copy link
Member

fredj commented Apr 8, 2015

externs/vbarray.js can be removed as well

@bartvde
Copy link
Member

bartvde commented Apr 8, 2015

+1 on the removal as well

@fredj
Copy link
Member

fredj commented Apr 8, 2015

@tschaub see https://github.com/tschaub/ol3/compare/unbinary...fredj:unbinary for the externs/vbarray.js and ol.format.FormatType.BINARY removal.

@elemoine
Copy link
Member

elemoine commented Apr 8, 2015

@fredj, @tschaub waited for #3481 for the removal of ol.format.FormatType.BINARY, to avoid conflicts.

@fredj fredj added this to the v3.5.0 milestone Apr 13, 2015
@fredj
Copy link
Member

fredj commented Apr 14, 2015

@tschaub You can find a rebased branch here: master...fredj:unbinary

@tschaub
Copy link
Member Author

tschaub commented Apr 18, 2015

Thanks for the extra commits @fredj. I think this is good to go now. Anybody else?

@tschaub
Copy link
Member Author

tschaub commented Apr 19, 2015

I'm taking previous comments as a sign that this is good to merge.

tschaub added a commit that referenced this pull request Apr 19, 2015
Remove ol.format.BinaryFeature.
@tschaub tschaub merged commit a67218b into openlayers:master Apr 19, 2015
@tschaub tschaub deleted the unbinary branch April 19, 2015 16:04
elemoine added a commit to openlayers/openlayers-closure-application that referenced this pull request Aug 3, 2015
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

6 participants