added schema validation and updated lint configuration #182

Merged
merged 1 commit into from Dec 21, 2016

Conversation

Projects
None yet
2 participants
@trescube
Contributor

trescube commented Dec 21, 2016

  • use Joi for sub-config schema validation, incorporated into download_data.js and import.js
  • copied .jslintrc from latest api

@trescube trescube added the in review label Dec 21, 2016

@trescube trescube added this to the API Improvements milestone Dec 21, 2016

@trescube trescube self-assigned this Dec 21, 2016

@orangejulius

looks great overall. did some other changes sneak in?

src/components/loadJSON.js
console.error('Inability to parse JSON usually means that WOF has been cloned ' +
'without using git-lfs, please see instructions here: ' +
'https://github.com/whosonfirst/whosonfirst-data#git-and-large-files');
- next(null, {});
+ next(parse_err);
}

This comment has been minimized.

@orangejulius

orangejulius Dec 21, 2016

Member

this all changes the behavior here. It's actually a change we want to make, but maybe it should be in a separate PR?

@orangejulius

orangejulius Dec 21, 2016

Member

this all changes the behavior here. It's actually a change we want to make, but maybe it should be in a separate PR?

This comment has been minimized.

@trescube

trescube Dec 21, 2016

Contributor

So make lint updates in a separate PR?

@trescube

trescube Dec 21, 2016

Contributor

So make lint updates in a separate PR?

This comment has been minimized.

@orangejulius

orangejulius Dec 21, 2016

Member

No, lint updates are fine to change. But the changes to this file change the behavior, because any error will now stop the stream. We want to do that (As mentioned in this comment), but it seems better to make that change in its own PR.

@orangejulius

orangejulius Dec 21, 2016

Member

No, lint updates are fine to change. But the changes to this file change the behavior, because any error will now stop the stream. We want to do that (As mentioned in this comment), but it seems better to make that change in its own PR.

This comment has been minimized.

@trescube

trescube Dec 21, 2016

Contributor

Lint is complaining about the long lines, that's why I changed them. I'll disable that in .jslintrc for now.

@trescube

trescube Dec 21, 2016

Contributor

Lint is complaining about the long lines, that's why I changed them. I'll disable that in .jslintrc for now.

added schema validation and updated lint configuration
use Joi for sub-config schema validation, incorporated into download_data.js and import.js
copied .jslintrc from latest api
@orangejulius

looks good now!

@trescube trescube merged commit 5683108 into master Dec 21, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@trescube trescube deleted the add-joi-config-validation branch Dec 21, 2016

@trescube trescube removed the in review label Dec 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment