Make main activity detection more solid

rnpm / rnpm-plugin-link

Allows linking dependencies to your React Native project

MIT License

31 stars 19 forks source link

Make main activity detection more solid #69

Closed geof90 closed 8 years ago

geof90 commented 8 years ago

Addresses https://github.com/rnpm/rnpm-plugin-link/issues/26

I completely removed the "getPrefix.js" file along with a ton of other code, because they are no longer needed, I hope you guys don't mind :p. There is now only a single "makeMainActivityPatch.js" that handles all the logic of detecting the type of MainActivity (whether it extends Activity or ReactActivity), and applying the appropriate patch.

grabbou commented 8 years ago

+30 -140

I love it! Thanks for that :) Waiting for @Kureev to dbl check and I think we can merge that in!

Kureev commented 8 years ago

I'm busy with refactoring patch system for Android right now. I think after that we can try to merge it in (but it'll require some additional efforts)

grabbou commented 8 years ago

Isn't this something you could try merging into your branch? (would be cool if you could submit some changes so we can check as you work!)

This looks like a small improvement and since it removes ~140 lines, it could save you a bit of time.

Kureev commented 8 years ago

@grabbou this wouldn't scale: https://github.com/rnpm/rnpm-plugin-link/pull/69/files#diff-9be25dfa687d08b7f584875b08c5b1feR11

We already have 3 different versions of MainActivity patch. This PR removes one of them and replace a versioning system by ternary check. That wouldn't work if tomorrow we'll have yet another breaking change in RN application structure.