web-push-libs / webpush-java

Web Push library for Java
MIT License
318 stars 112 forks source link

Update readme #74

Open condast opened 5 years ago

condast commented 5 years ago

Hi,

I have just spent a day trying to send notifications through WebPush, ad managed to get this done by specifically adding private and public keys to the PushService. Without this, I get authentication errors. This is not specifically mentioned in the ReadMe.md, so it may be worth adding this to the tutorial.

Thanks

Kees

martijndwars commented 5 years ago

Thanks for the feedback. I agree that it's not very clear from the README. Would you be willing to improve the text and create a PR? If not, I will work on this asap.

condast commented 5 years ago

Hi Martijn,

I'll look into it! I have OSGI-IFYD your code and am currently trying to see if I can upgrade it to a generic OSGO package. Maybe we can integrate the activities at some point. As far as I can currently tell, the public and private key ALWAYS need to be added to the push message, while the code suggests that this is optional, so I may suggest one or two minor refactorings in the near future to you!

Cheers

Kees

Op do 20 dec. 2018 om 10:12 schreef Martijn Dwars <notifications@github.com

:

Thanks for the feedback. I agree that it's not very clear from the README. Would you be willing to improve the text and create a PR? If not, I will work on this asap.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/web-push-libs/webpush-java/issues/74#issuecomment-448925936, or mute the thread https://github.com/notifications/unsubscribe-auth/ADUf9k61z4PvBgKji44BaF-hvCVT-yTHks5u61PLgaJpZM4Zauci .

-- Met vriendelijke groet,

Kees Pieters Condast www.condast.com

martijndwars commented 5 years ago

As far as I can currently tell, the public and private key ALWAYS need to be added to the push message, while the code suggests that this is optional,

The keys are not required if you do not use VAPID. Nowadays VAPID is the recommended approach, so this is mostly to stay backwards compatible.

Looking forward to your refactorings.

condast commented 5 years ago

ah..ok..then it is merely required to explicitly mention in the readme.md that these need to be added in a vapid situation. That makes sense!

Cheers

Kees

Op do 20 dec. 2018 om 15:44 schreef Martijn Dwars <notifications@github.com

:

As far as I can currently tell, the public and private key ALWAYS need to be added to the push message, while the code suggests that this is optional,

The keys are not required if you do not use VAPID. Nowadays VAPID is the recommended approach, so this is mostly to stay backwards compatible.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/web-push-libs/webpush-java/issues/74#issuecomment-449021169, or mute the thread https://github.com/notifications/unsubscribe-auth/ADUf9sFs96QYJkKByXQuLMsRk3xR1mUTks5u66HwgaJpZM4Zauci .

-- Met vriendelijke groet,

Kees Pieters Condast www.condast.com