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 date serializer #2

Merged
merged 1 commit into from
May 12, 2021
Merged

Fix date serializer #2

merged 1 commit into from
May 12, 2021

Conversation

parayab
Copy link
Member

@parayab parayab commented May 12, 2021

This PR changes the library behaviour, so it might break apps using it.

Before a Movement serialized like this:

amount: 7844717
currency: "CLP"
description: "Nihil aspernatur dignissimos est."
id: "mov_ZxENnDHvJAxreJkK"
postDate: {}
transactionDate: null
}

postDate and transactionDate, if not null, are serialized as empty objects.

Now a Movement is serialized like this:

amount: 7844717
currency: "CLP"
description: "Nihil aspernatur dignissimos est."
id: "mov_ZxENnDHvJAxreJkK"
postDate: "2021-05-11T00:00:00.000Z"
transactionDate: null
}

postDate and transactionDate, if not null, are serialized as strings.

Copy link
Member

@nateare nateare left a comment

Choose a reason for hiding this comment

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

LGTM

Before merging we should change the commit message to follow the same standard the public repos follow

@parayab parayab force-pushed the fix-date-serializer branch from 5d067cc to 84c35a9 Compare May 12, 2021 14:27
@parayab parayab changed the title fix(resources): fix date serializer Fix date serializer May 12, 2021
Copy link
Member

@lezorich lezorich left a comment

Choose a reason for hiding this comment

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

LGTM

@parayab parayab merged commit fc87504 into master May 12, 2021
@parayab parayab deleted the fix-date-serializer branch May 12, 2021 15:26
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.

4 participants