search

rnpm / rnpm-plugin-link

Allows linking dependencies to your React Native project
MIT License
31 stars 19 forks source link

Support adding/removing iOS shared libraries #89

Closed appden closed 8 years ago

appden commented 8 years ago

When removing shared libraries, only the ones not used by anyone else are removed.

The xcode module was updated to 0.8.8 for supporting tbd files (text-based dynamic library definitions).

This resolves rnpm/rnpm#122 and only works with rnpm/rnpm#172 merged, which provided the sharedLibraries array.

Kureev commented 8 years ago

@grabbou, can you take a look pls?

appden commented 8 years ago

FYI, this PR has a very minor conflict with the other PR of mine (#90) that was merged. It's a very obvious one to resolve, but if you prefer, I can rebase this branch on top of what's on master now.

grabbou commented 8 years ago

@appden no worries, we can rebase before merging! I have no nits here, looks very good. Great work!

I'll check it tomorrow (func wise) and see how's that working.

Do you think it's good to also test the addGroupWithMessage just for the sake of test coverage? The biggest issue with link function itself is that we haven't tested it, so now that we have moved part of it to a sep. file it might be a good idea!

appden commented 8 years ago

@grabbou I personally really don't think so since it simply composes getGroup and createGroup, both of which are tested.

Kureev commented 8 years ago

So, I think we need to push this one forward :)

grabbou commented 8 years ago

As I already answered, I'll merge it tomorrow :)

grabbou commented 8 years ago

Merged, I've also added one commit that handles missing Frameworks group during unlink that I've run into while testing on a blank app.

Apart from that - great, all builds and works nice! Thanks!

grabbou commented 8 years ago

Took me a while, but had to rebase merge commit and change the author to @appden since the suggested merge & rebase by Git completely ignores the author :) Now all good.