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

Fixing Bug: inconsistent date formatting in feed listing #42

Merged
merged 2 commits into from Oct 4, 2018

Conversation

Projects
None yet
2 participants
@alimasyhur
Copy link
Contributor

alimasyhur commented Oct 3, 2018

Fixes #39

Fixes Bug inconsistent date formatting in feed listing.

I added default time format, and let the time format in feed listing follows it

@billglover
Copy link
Owner

billglover left a comment

I've tested this PR and all looks good, many thanks. I've left one comment on the location of the package level variable. If you can address this, we should be good to merge.

@@ -16,6 +16,8 @@ var listFeedCmd = &cobra.Command{
Run: listFeed,
}

var defaultTimeFormat = "2006-01-02 15:04:05.000 +0000 MST"

This comment has been minimized.

@billglover

billglover Oct 3, 2018

Owner

As this line defines a package level variable, it's easy to miss given that it is in the cmd/listFeed.go file. Ideally this would be passed as a configuration parameter but doing so is outside the scope of this fix. A compromise would be to move this line to cmd/root.go so at least it is co-located with other configuration items.

This comment has been minimized.

@alimasyhur

alimasyhur Oct 4, 2018

Author Contributor

sure, I moved it to root.go file

@alimasyhur
Copy link
Contributor Author

alimasyhur left a comment

sure, I moved defaultTimeFormat to root.go file

@@ -16,6 +16,8 @@ var listFeedCmd = &cobra.Command{
Run: listFeed,
}

var defaultTimeFormat = "2006-01-02 15:04:05.000 +0000 MST"

This comment has been minimized.

@alimasyhur

alimasyhur Oct 4, 2018

Author Contributor

sure, I moved it to root.go file

@billglover
Copy link
Owner

billglover left a comment

Changes look good. Many thanks.

@billglover billglover merged commit 113eba3 into billglover:master Oct 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment