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

Add @api to ol.Sphere #2641

Merged
merged 4 commits into from Aug 29, 2014
Merged

Add @api to ol.Sphere #2641

merged 4 commits into from Aug 29, 2014

Conversation

elemoine
Copy link
Member

Adding @api to ol.Sphere makes it appear in the API docs, and provides a link from ol.geom.Polygon#circular. As commented in #2359 this does not help with ol.Sphere.WGS84, but that problem can be handled separately.

Adding @api to ol.Sphere also fixes a problem in the externs file generated by the generate-externs.js task. Without the @api annotation the externs file includes references to the ol.Sphere, which is unknown.

Please review.

@probins
Copy link
Contributor

probins commented Aug 27, 2014

this doesn't really close #2359, as there's still no docs on how to use Sphere, which was what that issue was mainly about. A simple workaround would be to add some docs with an example on how to use ol.sphere.WGS84 to ol.Sphere.

@elemoine
Copy link
Member Author

this doesn't really close #2359, as there's still no docs on how to use Sphere, which was what that issue was mainly about. A simple workaround would be to add some docs with an example to ol.Sphere.

You're right. Removing the "closes" annotation.

@probins probins mentioned this pull request Aug 29, 2014
@elemoine
Copy link
Member Author

@ahocevar do you agree with merging this?

@elemoine elemoine added this to the v3.0.0 milestone Aug 29, 2014
@ahocevar
Copy link
Member

I do, but at least 1-2 sentences of documentation would be nice.

@elemoine
Copy link
Member Author

Good call. I made other changes. The tissot example now uses the documented ol.Sphere class directly, as opposed to using the ol.sphere.WGS84 instance which is not in the documentation (as reported in #2359). So I also removed the @api annotation from ol.sphere.WGS84. This time I think this PR fully addresses #2359. Please review.

@ahocevar
Copy link
Member

Looks great to me, but it seems Travis does not agree.

@elemoine
Copy link
Member Author

Yep, I think it's now fixed.

@elemoine
Copy link
Member Author

Closes #2359.

* Class to create objects that can be used with {@link
* ol.geom.Polygon.circular}.
*
* For example to create a sphere whose radius equal to the semi-major
Copy link
Contributor

Choose a reason for hiding this comment

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

whose radius is equal to

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. Thanks.

@probins
Copy link
Contributor

probins commented Aug 29, 2014

I agree that this resolves #2359

elemoine pushed a commit that referenced this pull request Aug 29, 2014
@elemoine elemoine merged commit 5df0b4e into openlayers:master Aug 29, 2014
@elemoine elemoine deleted the sphere branch August 29, 2014 10:10
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