Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HMAC Push notification updates #283

Merged
merged 5 commits into from
Mar 14, 2024
Merged

HMAC Push notification updates #283

merged 5 commits into from
Mar 14, 2024

Conversation

nplasterer
Copy link
Contributor

@nplasterer nplasterer commented Mar 12, 2024

Adding the new HMAC code to the example and push classes to help make this easier.

See typescript docs here: https://github.com/xmtp/example-notification-server-go/blob/main/docs/notifications-client-guide.md

@nplasterer nplasterer self-assigned this Mar 12, 2024
@nplasterer nplasterer marked this pull request as ready for review March 12, 2024 04:37
@nplasterer nplasterer requested a review from a team as a code owner March 12, 2024 04:37
@zombieobject
Copy link
Contributor

zombieobject commented Mar 12, 2024

@nplasterer I am currently looking into the build for test failures. From what I can see in the logs, the pipeline never gets to the test stage. I will update once I figure out the source. Going to switch to local builds to isolate it for now.

@zombieobject zombieobject added the on-hold Do not merge. This PR is on hold. label Mar 12, 2024
Copy link
Contributor

@zombieobject zombieobject left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nplasterer Something is off with the protobuf code generation in service.connect.swift that is causing build errors. Additionally, build for test fails as well, but I can't resolve those until the protobuf issue is corrected. See review comments inline for further details and LMK how I can help. 👨🏼‍🚒

Sources/XMTPiOS/Push/service.connect.swift Show resolved Hide resolved
Sources/XMTPiOS/Push/service.connect.swift Show resolved Hide resolved
Sources/XMTPiOS/Push/service.connect.swift Show resolved Hide resolved
Sources/XMTPiOS/Push/XMTPPush.swift Outdated Show resolved Hide resolved
Also update connect-swift package version.
@zombieobject zombieobject removed the on-hold Do not merge. This PR is on hold. label Mar 12, 2024
@zombieobject zombieobject self-requested a review March 12, 2024 21:05
Copy link
Contributor

@zombieobject zombieobject left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes by @kele-leanes to update the connect-swift package addressed the protobuf questions. Additionally, some rearrangement of the if/def fencing in XMTPPush.swift fixed the typealias visibility when building for test in CI.

@nplasterer
Copy link
Contributor Author

I was hoping to have more time to validate this this week but going to merge in favor unblocking integrators and will follow up with any tweaks once I've fully validated on Android.

@nplasterer nplasterer merged commit 89b62b4 into main Mar 14, 2024
2 checks passed
@nplasterer nplasterer deleted the np/hmac-push-notifications branch March 14, 2024 18:23
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