Skip to content

Conversation

@FredericRuaudel
Copy link
Contributor

In order to test my model properties, I need to access the properties values of the APNs data models.

As they are already public in the Android side, I imagine this is not a deal breaker to homogenize it on the Apple side?

I just have a few doubts on the final result:

  1. should we make them accessible only in readonly mode with a private(set) (I didn't add it since Android is fully public)
  2. To make this work, I had to make FCMApnsAlertOrString public to make Swift compiler happy in FCMApnsApsObject (aka: can't set property public because type used in not public yadiyada)
  3. Not sure on the naming of the FCMApnsAlertOrString helpers to access the enum associated values: alertPayload/alertMessage or asPayload/asMessage or asPayload/asString

at call site it looks like this right now:

iosConfig?.payload.aps.alert?.alertPayload?.titleLocArgs

so maybe it would look better like this:

Config?.payload.aps.alert?.asPayload?.titleLocArgs

🤔

What do you think?

Thanks in advance!

@MihaelIsaev MihaelIsaev merged commit bf42d59 into MihaelIsaev:master Nov 28, 2019
@MihaelIsaev
Copy link
Owner

@FredericRuaudel thank you for the pull request! ❤️

  1. Maybe it'd look better, yeah 🙂 but we definitely should keep old way too just by marking it deprecated to save backward compatibility

@FredericRuaudel
Copy link
Contributor Author

Maybe it'd look better, yeah 🙂 but we definitely should keep old way too just by marking it deprecated to save backward compatibility

I'm not sure we understand each other here, I've added those computed properties so I could have change the name right now and no need for deprecated marker. Or I didn't get what you said? 🤔

@MihaelIsaev
Copy link
Owner

Maybe it'd look better, yeah 🙂 but we definitely should keep old way too just by marking it deprecated to save backward compatibility

I'm not sure we understand each other here, I've added those computed properties so I could have change the name right now and no need for deprecated marker. Or I didn't get what you said? 🤔

Yeah, my bad, sorry I was in a hurry 🙂 no questions! all good! 🙂

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