Skip to content
New issue

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

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

Already on GitHub? # to your account

✨ Feature/18 #29

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨ Feature/18 #29

wants to merge 1 commit into from

Conversation

abhissng
Copy link

This PR adds the feature to add custome time format for JSON logs

**This PR added new feature #18 **

With this feature users shall be able to define the timezone and time format to get the desired time in the JSON logs

@abhissng abhissng changed the title ✨ Feature/18 #18 ✨ Feature/18 Oct 27, 2022
@alessandroargentieri
Copy link
Member

Thanks for your PR. I've seen no tests added. You can use the format already present in the logger_test.go in which by passing a byte buffer to the logger you can get the string logged and verify that the timing format chosen corresponds to the one printed (i.e. https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-date-and-time-format-strings)

The code coverage is 99% so it would be nice to keep this "record" for a simple logger.

@abhissng
Copy link
Author

Hi @alessandroargentieri Thank you for the feedback.

I can add the test case for the format based on the hard coded expected date, but i am not sure how can i match the time value which will change every other second.

  • Let say i have the expected value as "2022-10-31 22:04:58.1233"
  • once i run func getTime() the string which will be received can have the value as
  • "2022-10-31 22:04:59.4564" or "2022-10-31 22:05:01.434"

need your suggestion if i can add the test case for other format where this time uniqueness is not there. If yes i'll create the PR for the same.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants