search

TylerBrock / saw

Fast, multi-purpose tool for AWS CloudWatch Logs
MIT License
1.41k stars 79 forks source link

Support more time formats #27

Closed andrewpage closed 5 years ago

andrewpage commented 5 years ago

Useful tool, first of all. Thanks for writing it.

Currently the time parsing for the --start and --end parameters is limited to the restrictive and annoying time.RFC3339. This change allows the program to try multiple time formats until one matches.

I've also added some basic tests for the getTime method.

Fixes https://github.com/TylerBrock/saw/issues/16 and https://github.com/TylerBrock/saw/issues/26

TylerBrock commented 5 years ago

This is great, thank you. Sorry it's taken so long to review. I've been on vacation.

Also, thank you for writing tests! One quick question though: any way to clean the tests up a bit by breaking up the long lines?

andrewpage commented 5 years ago

Thanks for the review @TylerBrock. I've re-formatted the test code into shorter lines. Let me know what you think.

Andrew

TylerBrock commented 5 years ago

Awesome, last thing: In the tests, do you think there is a way to create the expected time objects and use time.Time.Equal() to test equality? Might clean up the tests a bit more.

Sorry this wasn't in my initial review, it didn't occur to me that you could do that until later.

andrewpage commented 5 years ago

Done! I rarely program in Go, so I wasn't familiar with that one either. Let me know if you see any other improvements.

Also, another thing @TylerBrock. What do you think about getting something like Travis CI set up for pull request reviews? I think it's free for open source projects. Would be nice to see test results in the pull request checks.

TylerBrock commented 5 years ago

I am definitely a fan of TravisCI/CircleCI for testing projects. You've contributed the first tests so I hadn't set it up but I think that is the next thing on my TODO list, hah. Thanks again for all the hard work on this great contribution and patience waiting for me to look at it.