Skip to content

Conversation

@BRUHItsABunny
Copy link

@BRUHItsABunny BRUHItsABunny commented Nov 25, 2022

Before opening your pull request, please make sure that you've:

  • updated the changelog if the change is user-facing;
  • added tests to cover your changes;
  • run the test suite locally (make test); and finally,
  • run the linters locally (make lint).

Thanks for your contribution!

@CLAassistant
Copy link

CLAassistant commented Nov 25, 2022

CLA assistant check
All committers have signed the CLA.

@BRUHItsABunny
Copy link
Author

BRUHItsABunny commented Nov 25, 2022

Fixes #124

Copy link
Collaborator

@prashantv prashantv left a comment

Choose a reason for hiding this comment

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

I think we should implement MarshalText instead, since the JSON encoding of a time is a string, and MarshalText will work in more cases, and matches atomic.String better.

@BRUHItsABunny
Copy link
Author

Agreed, that does make more sense.
Will make the change shortly.

@BRUHItsABunny
Copy link
Author

@prashantv My apologies for the delay, I've updated the PR with the latest comments in mind.

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