search

Ph3nol / NotificationPusher

Standalone PHP library for easy devices notifications push.
MIT License
1.18k stars 273 forks source link

Remove check on token length as this can vary #193

Closed johnboy-leeds closed 4 years ago

johnboy-leeds commented 4 years ago

As per issue: https://github.com/Ph3nol/NotificationPusher/issues/185

The length can vary and Apple advises against hardcoded lengths: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/APNSOverview.html#//apple_ref/doc/uid/TP40008194-CH8-SW14

seyfer commented 4 years ago

@johnboy-leeds would be great if it is covered with the test.

jonmilsom commented 4 years ago

@seyfer as the code change is removing the length requirement there are no changes to the test suite.

Had there been a separate assertion which tested the absolute length requirement, this would have been removed in this PR.

If I have misunderstood, please advise as to what test changes should be made, otherwise please consider merging this PR 😄

Thanks! 👍

seyfer commented 4 years ago 
Had there been a separate assertion which tested the absolute length requirement, this would have been removed in this PR.

A test can be added in a way, that test will check - if longer then 64 characters key passed, no exception was produced, for example.

seyfer commented 4 years ago

@jonmilsom here I have added the missing test https://github.com/Ph3nol/NotificationPusher/pull/194

jonmilsom commented 4 years ago

Thank you @seyfer 👍