Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Invalid tag modification v2 #149

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

bkowshik
Copy link
Contributor

Uses learning's from #147 to implement:

  1. When a feature has two or more primary tags and if one is removed, it is a 🆗
  2. When all properties of a feature are removed, assume feature is being moved to a relation

@amishas157 the PR also has a script to scrape data and a Jupyter notebook with all the analysis. It is ok if you review just the comparator and tests.

@bkowshik bkowshik requested a review from amishas157 April 11, 2017 11:14
@batpad
Copy link
Contributor

batpad commented May 9, 2017

@bkowshik @amishas157 - is this good to merge? any next actions?

@amishas157
Copy link
Contributor

Sorry @bkowshik , got late for reviewing this PR. Will be reviewing this PR by today and see the next actions here.

@@ -22,10 +22,14 @@ function invalidTagModification(newVersion, oldVersion, callback) {
var result = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

@bkowshik 👋 ,instead of returning {} for a false result, we are now following convention to send false value itself. Ref commit: 756c5cb

@@ -22,10 +22,14 @@ function invalidTagModification(newVersion, oldVersion, callback) {
var result = {};
if (!newVersion || !oldVersion) return callback(null, result);
Copy link
Contributor

Choose a reason for hiding this comment

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

Now we assume that newVersion will always be present in a geojson. And if a feature is deleted, we will check for deleted tag of new version. Ref commit : 2e0e6b6

So (!newVersion || !oldVersion) will become (newVersion.deleted || !oldVersion)

@amishas157
Copy link
Contributor

When a feature has two or more primary tags and if one is removed, it is a 🆗

@bkowshik By that did you mean should we catch feature only when its one and only primary tag is deleted from it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants