intercom-react-native icon indicating copy to clipboard operation
intercom-react-native copied to clipboard

Published 20 hours ago • verified publisher icon intercom

fix: backwards compatibility with older AGPs

Open looskie opened this issue 1 year ago • 5 comments

As pointed out by @filipef101, I accidentally removed the namespace here.

Furthermore, this entire patch for newer AGPs (Including this PR, and #152) is actually not even needed anymore considering RN 0.73.3 (Not 0.73.2/0.73.1 though) adds backwards compatibility for packages to use namespace. but, it doesn't hurt to still have it!

looskie avatar Feb 26 '24 16:02 looskie

Thank you. Sorry for tangent, it's very hard to communicate as the repo doesn't allow for issues, and instead focus on intercom community forum which adds a lot of friction. That said, could you also take a look at https://github.com/intercom/intercom-react-native/pull/163

I would also make a suggestion of adding CI flows to build the example app and e2e test, even if just app launch success

filipef101 avatar Feb 27 '24 11:02 filipef101

Thank you. Sorry for tangent, it's very hard to communicate as the repo doesn't allow for issues, and instead focus on intercom community forum which adds a lot of friction. That said, could you also take a look at #163

I would also make a suggestion of adding CI flows to build the example app and e2e test, even if just app launch success

No problem at all! I agree, I get why they disable issues, but it does add a ton of friction.

That said, could you also take a look at #163 for the record, I don't work at intercom, just an open source contributor trying to help out the community.

The PR looks fine though, if its installing in that path that the author referenced i think it LGTM. Obviously testing would be a better idea than just looking though ;). Will test it if I get the time!

looskie avatar Feb 27 '24 14:02 looskie

I'm marking this a draft for now as I clearly need to test it. Will test when I get the bandwidth

looskie avatar Mar 06 '24 02:03 looskie

Maybe just merge the namespace? Or that breaks something? @Looskie

filipef101 avatar Mar 07 '24 12:03 filipef101

Maybe just merge the namespace? Or that breaks something? @Looskie

Sorry for the late reply, it would break versions 0.73.0 0.73.1 and 0.73.2. 0.73.3 and higher infer these changes by default for backwards compatibility.

I'll make some time this weekend or maybe during the evening to make a permanent solution.

looskie avatar Mar 12 '24 14:03 looskie