Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification Operations #1

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

gsedubun
Copy link

hi, i have modified and include suport for Register Notification. if you don't mind i would like to contribute back.

thanks alot.

@tipstrade
Copy link
Collaborator

Hi Gadael, many thanks. I'll have a look at this, but I'm not going to merge as-is as the code style is quite different, plus I think the implementation can be improved a little.

My preferred method would be to expose Create, Add, Remove methods to the user. Also a bit of refactoring is desirable to remove the copypasta.

Just to be sure, are you using this as the reference?

I've also noticed that you've hard coded in your project's Sender ID.

@gsedubun
Copy link
Author

yup, i was trying to do token refreshing to send messages to one or more devices.
you got me on the hardcoded project sender ID and copypasta. i'll fix it and comeback at you again.
thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants