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

Reduced use of the vgl module. #918

Merged
merged 1 commit into from
Sep 11, 2018
Merged

Reduced use of the vgl module. #918

merged 1 commit into from
Sep 11, 2018

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Sep 4, 2018

The object and timestamp classes were using the vgl module definitions of these as base classes. This obscured documentation -- the modified and getMTime methods were not in the geojs code, and therefore did not appear in the jsdocs. Rather than artificially inject it, this simplifies code to move that functionality to the geojs repo. This means that only the gl renderer has any dependency on the vgl module (except fir the clustering code, which needs to be refactored in any case).

As part of this, getMTime was renamed to the somewhat less obscure timestamp. The old name is still supported, but not in the jsdoc (it wasn't in the jsdoc before, either).

This also converts all uses of new Date().getTime() to Date.now().

Copy link

@johnkit johnkit left a comment

Choose a reason for hiding this comment

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

OK w/changelog fix

The `object` and `timestamp` classes were using the vgl module
definitions of these as base classes.  This obscured documentation --
the `modified` and `getMTime` methods were not in the geojs code, and
therefore did not appear in the jsdocs.  Rather than artificially inject
it, this simplifies code to move that functionality to the geojs repo.
This means that only the gl renderer has any dependency on the vgl
module (except fir the clustering code, which needs to be refactored in
any case).

As part of this, `getMTime` was renamed to the somewhat less obscure
`timestamp`.  The old name is still supported, but not in the jsdoc (it
wasn't in the jsdoc before, either).

This also converts all uses of `new Date().getTime()` to `Date.now()`.
@manthey manthey merged commit c681b2f into master Sep 11, 2018
@manthey manthey deleted the reduce-vgl-use branch September 11, 2018 17:27
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