apache / cordova-plugman

Apache Cordova Plugman
https://cordova.apache.org/
Apache License 2.0
399 stars 151 forks source link

Correctly tell plugman which object in config to remove #26

Closed mbillau closed 10 years ago

mbillau commented 10 years ago

CB-5016: Plugman was always assuming that a plugin found as a dependency was installed as a dependency. This led to a problem where explicitly adding a dependent plugin manually would lead to a broken state after removing the "parent" plugin because the dependent plugins entry would still be in the plugins/{platform}.json file. The change just checks to see if the plugin to be deleted is listed as a dependency_plugin or an installed_plugin and sets "is_top_level" so that the entry is correctly removed from {platform}.json. Tested with npm test, no failures.

bshepherdson commented 10 years ago

Other than the quirk I commented on above, this change LGTM and I'll be happy to merge it. Good find.

bshepherdson commented 10 years ago

Tweaked and merged as d4a9e89302f78a4377ddb00c80258e1edd965ccc .

mbillau commented 10 years ago

Thanks a lot Braden. I see it was merged in as d4a9e893 so I'll close this pull request and the Jira item. Good point on the p ? true : false === p issue, I'll remember that one.