feat(Platform Notifications): add new service to the project#335
feat(Platform Notifications): add new service to the project#335AkosSz wants to merge 7 commits into
Conversation
|
|
|
Do not merge yet |
pyrooka
changed the title
pyrooka
left a comment
pyrooka
left a comment
There was a problem hiding this comment.
You need to run the detect secrets tool through the project and update the .secrets.baseline file to make our CI happy. :) Also, please attach a screenshot of a clean examples test run too. Other than that, the changes are in a pretty good shape!
|
example test runs: were you thinking about adding the detect secrets as a pre-commit hook? 😅 |
Nope, just updating the |
|
test results for the latest commit: example tests: |
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
Signed-off-by: Ákos Sztremi <Akos.Sztremi@ibm.com>
pyrooka
left a comment
pyrooka
left a comment
There was a problem hiding this comment.
I left a small comment that would be nice to address before merging, but I approve this PR since it looks good overall and this way we can avoid the re-review.
| // PLATFORM_NOTIFICATIONS_URL=<service base url> | ||
| // PLATFORM_NOTIFICATIONS_AUTH_TYPE=iam | ||
| // PLATFORM_NOTIFICATIONS_APIKEY=<IAM apikey> | ||
| // PLATFORM_NOTIFICATIONS_AUTH_URL=<IAM token service base URL - omit this if using the production environment> |
There was a problem hiding this comment.
Could you update this list to contain all the necessary config properties?
3 participants
PR summary
PR Checklist
Please make sure that your PR fulfills the following requirements:
Current vs new behavior
Platform notifications integration added to the sdk.
Does this PR introduce a breaking change?
Other information
Integration Test results
unit test results