Updated warning for boundary.circle #617

Merged
merged 2 commits into from Aug 9, 2016

Conversation

Projects
None yet
3 participants
@avulfson17
Contributor

avulfson17 commented Aug 3, 2016

Fixes (or at least modifies) #510

@dianashk dianashk added the in progress label Aug 3, 2016

@orangejulius

This comment has been minimized.

Show comment
Hide comment
@orangejulius

orangejulius Aug 3, 2016

Member

LGTM!

Member

orangejulius commented Aug 3, 2016

LGTM!

@dianashk dianashk added in review and removed in progress labels Aug 4, 2016

@@ -50,7 +50,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) {
// t.equals(clean['boundary.circle.radius'], 12.121212, 'should be set to point.lat')
t.deepEquals(errorsAndWarnings, {
errors: [],
- warnings: ['boundary.circle is currently unsupported']
+ warnings: []

This comment has been minimized.

@dianashk

dianashk Aug 4, 2016

Contributor

this test is explicitly checking for a warning that boundary.circle.radius is going to be ignored, which it is. the code changes you've made don't (or shouldn't) change that behavior. this test is actually proving that the code is broken.

@dianashk

dianashk Aug 4, 2016

Contributor

this test is explicitly checking for a warning that boundary.circle.radius is going to be ignored, which it is. the code changes you've made don't (or shouldn't) change that behavior. this test is actually proving that the code is broken.

This comment has been minimized.

@dianashk

dianashk Aug 4, 2016

Contributor

sorry, ignore the first comment. i was reading this wrong.

however, the name of the test should be updated to reflect what the new expectation is.

@dianashk

dianashk Aug 4, 2016

Contributor

sorry, ignore the first comment. i was reading this wrong.

however, the name of the test should be updated to reflect what the new expectation is.

@dianashk

This comment has been minimized.

Show comment
Hide comment
@dianashk

dianashk Aug 9, 2016

Contributor

:shipit:

Contributor

dianashk commented Aug 9, 2016

:shipit:

@orangejulius orangejulius merged commit 3ff4598 into master Aug 9, 2016

3 checks passed

approvals/lgtm this commit looks good
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@orangejulius orangejulius removed the in review label Aug 9, 2016

@orangejulius orangejulius deleted the boundary-circle-warning branch Aug 19, 2016

je-l pushed a commit to nlsfi/pelias-api that referenced this pull request Aug 31, 2017

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