Replace get/set functions with properties : Core #1573
Conversation
| defineProperties(EllipsoidGeodesic.prototype, { | ||
| /** | ||
| * The surface distance between the start and end point | ||
| * @memberof EllipsoidGeodesic.prototype |
There was a problem hiding this comment.
Throughout, @memberof doesn't need the .prototype, right?
There was a problem hiding this comment.
I was told in other pull request to do it this way
There was a problem hiding this comment.
This pull request has it both ways, e.g., see endHeading below. The Model code also doesn't have .prototype on properties. Whichever generates the right doc is OK with me.
There was a problem hiding this comment.
Anything inside of a defineProperties block needs to have the .prototype. Things defined directly on the object in the constructor function do not. That's why Model works as it does. In reality we need to update JSDoc as I'm sure there are plenty of fixes to these quirks, @shunter said the latest version can do our entire code base in under a minute. Whatever pain we have to go through to update will be worth it.
There was a problem hiding this comment.
Ah, OK. At closer look, ModelAnimationCollection.length is static. Will submit a separate issue.
|
Tests and Sandcastle are good. Just those minor comments. |
|
@pjcozzi Should i change Other than that, this should be ready to go. |
Sure. |
|
@pjcozzi ready |
Replace get/set functions with properties : Core
#1421
Occluder, LeapSecond, Fullscreen, Event, EllipsoidGeodesic