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

Fix gcp format #198

Merged
merged 10 commits into from
Aug 28, 2024
Merged

Fix gcp format #198

merged 10 commits into from
Aug 28, 2024

Conversation

lassemand
Copy link
Collaborator

@lassemand lassemand commented Aug 28, 2024

Purpose

https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#Message

Data only allows for String and String.

This means we need to take ownership of some of the types

Changes

Changed serialisation for those fields which did not return a string

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@lassemand lassemand requested a review from soerenbf August 28, 2024 12:09
@lassemand lassemand requested a review from Radiokot August 28, 2024 12:19
@lassemand lassemand merged commit 984a921 into main Aug 28, 2024
4 checks passed
@lassemand lassemand deleted the lma/fix/gcp_format branch August 28, 2024 12:33
@@ -397,7 +397,7 @@ mod tests {
"amount": "200",
"type": "cis2-tx",
"token_id": "ffffff",
"contract_address": {"index": 3, "subindex": 0},
"contract_address": "{\"index\":3,\"subindex\":0}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Lame... but if that's what it has to be, I guess there's no way around it 🤷

Copy link
Member

Choose a reason for hiding this comment

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

Wasn't it you who suggested using JSON for contract address? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes but I would assume the whole request being JSON would be sufficient 😛

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.

3 participants