robcalcroft / react-native-in-app-notification

:bell: Customisable in-app notification component for React Native
MIT License
270 stars 78 forks source link

Major rewrite #16

Closed panz3r closed 6 years ago

panz3r commented 6 years ago

Rewrite module to be fully compatible with latest React Native releases (0.54 and later) and to take advantage of new Context API to simplify integration.

Notable changes:

robcalcroft commented 6 years ago

Noticed you've got a couple of other PRs open here, do they need reviewing too? Looks like some overlap.

panz3r commented 6 years ago

Noticed you've got a couple of other PRs open here, do they need reviewing too? Looks like some overlap.

Hi @robcalcroft , You can safely ignore those others PR since those are included into this one. I made them before coming up with the idea of updating the whole module 😬

robcalcroft commented 6 years ago

Sure thing, could you close them off then pls 😊

On 7 Oct 2018, at 16:01, Mattia Panzeri notifications@github.com wrote:

Noticed you've got a couple of other PRs open here, do they need reviewing too? Looks like some overlap.

Hi @robcalcroft , You can safely ignore those others PR since those are included into this one. I made them before coming up with the idea of updating the whole module 😬

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

panz3r commented 6 years ago

Hi @robcalcroft , I made all the changes as you suggested.

robcalcroft commented 6 years ago

If you could please use the rebase option instead of the merge commit option that would be great, not keen on merge commits

panz3r commented 6 years ago

If you could please use the rebase option instead of the merge commit option that would be great, not keen on merge commits

Hi @robcalcroft, I don't have write access to this repo so I can't rebase or merge this PR, I suppose this is up to you 👍

robcalcroft commented 6 years ago

Ended up merging cos rebase would've needed some more faffing 😄