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

Adds apns setter #277

Closed
wants to merge 2 commits into from
Closed

Adds apns setter #277

wants to merge 2 commits into from

Conversation

grahac
Copy link

@grahac grahac commented Sep 24, 2024

adds a set_apns call. I am totally fine with you doing it a different way but this works!!!!

@hpopp
Copy link
Member

hpopp commented Sep 24, 2024

I appreciate you using the package!

So at first glance, this is functionally no different from:

notification
|> Map.put(:apns, apns)

I'm open to the idea of well-named setters for FCM notifications, but unlike APNS.Notification, they wouldn't be doing much (except maybe type validations). Is there interest in having them? We'd need to have them defined for all of the FCM notification attributes, along with proper typespecs and docs.

@grahac
Copy link
Author

grahac commented Sep 26, 2024

Well duh. For some reason I I had it in my head that you couldn't just set the Map.put externally. My suggestion is just to show some examples of this in the readme and call it a day. Changing my source code now too :)

@grahac grahac closed this Sep 26, 2024
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