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

Set centered param default to True #836

Merged
merged 2 commits into from
Jul 12, 2017
Merged

Conversation

stebogit
Copy link
Collaborator

Fixes #833.

@DenisCarriere I added a note in the doc that notify/warn the user that centered will be dropped in the next major release, to try to discourage its use.
Is that ok?

simplified tests;
updated all relevant files;
@@ -3,6 +3,6 @@ import {BBox, Points, Units, Feature, Features} from '@turf/helpers';
/**
* http://turfjs.org/docs/#pointgrid
*/
declare function pointGrid(bbox: BBox | Feature<any> | Features<any>, cellSide: number, units?: Units, centered?: boolean, bboxIsMask?: boolean): Points;
declare function pointGrid(bbox: BBox | Feature<any> | Array<number>, cellSide: number, units?: Units, centered?: boolean, bboxIsMask?: boolean): Points;
Copy link
Member

@DenisCarriere DenisCarriere Jul 12, 2017

Choose a reason for hiding this comment

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

  1. Why the removal of Features<any> this is to support FeatureCollection, I guess we should use the full name of the Type (FeatureCollection<any>) instead of making the types plural.

  2. BBox is already defined - Array<number> is not needed

BBox => [number, number, number, number]

CC: @stebogit

Copy link
Member

@DenisCarriere DenisCarriere Jul 12, 2017

Choose a reason for hiding this comment

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

I need to take some time and update all the Typescript definitions for each module (all 85 of them.. 😱) make them all consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DenisCarriere it'd be nice if we had some kind of types file (TYPES.md?) maybe in the root as reference for all Turf modules. With that I could help updating all the modules and would be useful, when creating new models, for who like me doesn't know typescript

Copy link
Member

Choose a reason for hiding this comment

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

That would be a good idea, maybe I'll create that before updating all the definitions.

My approach to Typescript is not as strict as others like DefinitelyTyped which enforce strict linting (don't think we are there yet for TurfJS - might add a basic TS linting config).

@DenisCarriere DenisCarriere added this to the 4.6.0 milestone Jul 12, 2017
@DenisCarriere DenisCarriere merged commit 7840163 into master Jul 12, 2017
@DenisCarriere DenisCarriere deleted the point-grid-centered branch July 12, 2017 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants