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

tag_mapper: skip inherited enumerable properties #558

Merged
merged 2 commits into from
Aug 11, 2021
Merged

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Aug 10, 2021

I was interested in #557, it seems that we are using the in operator / for ... in syntax which iterates over all properties "including inherited enumerable properties".

A quick git grep shows other usages of that syntax in this lib which might need attention too.

closes #557

@orangejulius
Copy link
Member

Nice, I haven't touched this code in a while so I wasn't quite sure where the fix would end up. Looks like the right place. If you think any of the other for ... in code needs fixing, go for it. Otherwise this looks good to merge 🚢

@missinglink
Copy link
Member Author

missinglink commented Aug 11, 2021

alrighty, not the most enjoyable task of the week 😆 but I've added 1c9b836 which replaces all usages of the in operator / for ... in syntax.

the nice thing about this repo is we have pretty decent unit test coverage and the end-to-end tests which helped me resolve any issues.

@missinglink missinglink merged commit d952ea4 into master Aug 11, 2021
@missinglink missinglink deleted the native_code branch August 11, 2021 09:32
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.

OSM records with 'constructor' tag result in invalid Pelias documents
2 participants