PushAsyncService #139
PushAsyncService #139
Conversation
|
Hmm, looks like the CI fails on |
| sourceCompatibility = 1.7 | ||
| targetCompatibility = 1.7 | ||
| sourceCompatibility = 1.8 | ||
| targetCompatibility = 1.8 |
There was a problem hiding this comment.
I'm okay with replacing the minimum requirement from Java 7 to Java 8. Can you change this in README.md as well (first line)?
|
Sorry for the delay, I've been on vacation for the last two weeks. I've tried to revive the integration tests in testcafe, but I haven't managed to get these to work on Travis CI. The integration tests have been a huge pain point (for all projects in the web-push-libs organization, as far as I'm aware). I'll run some manual tests when I'm back home. I don't expect any problems and I'll merge and release a new version soon after. |
Sure, no worries, thanks for your time :) |
|
Hey @martijndwars, any update for this PR? Did you get a chance to run manual tests yet? |
|
Yep, ran the tests a while back but forgot to merge. Great work! |
|
when do you think we could get this in a new release? |
|
I published version 5.1.1 last night (CC @sp00m @fecheromero). |
Introducing
PushAsyncServicefor #101.I tried to keep the existing coding style and to ensure backward compatibility.
Still WIP because of the following points though:
CompletableFutureis only available since Java 8, and I did not notice this project still supports Java 7. I've update to Java 8, is that OK?I've kept the existing
PushServiceand introducedPushAsyncServiceto ensure backward compatibility. The shared code has been moved into a parent classAbstractPushService. Because the setters returnthisto allow chaining, I had to hack around with the usual<T extends AbstractPushService<T>>, which ends up in "unchecked cast" (although safe in this specific case), is that OK?I've deprecated
PushService#sendAsync, redirecting toPushAsyncService#sendinstead, is that OK?I've been using AHC, but I noticed that only 13 days ago, it is now looking for a new maintainer... I hope it will find one, but in the meantime, is that OK?
I wanted to add integration tests around
PushAsyncService, but I didn't manage to run the existing ones locally... Any specific config I'm supposed to set up first?Let me know :)