Skip to content

Add basic format option#114

Merged
dominikbraun merged 2 commits intodominikbraun:mainfrom
sfrique:issue-109/add-format
Jun 12, 2021
Merged

Add basic format option#114
dominikbraun merged 2 commits intodominikbraun:mainfrom
sfrique:issue-109/add-format

Conversation

@sfrique
Copy link
Copy Markdown
Contributor

@sfrique sfrique commented Jun 11, 2021

Initial very basic implementation for #109

I didn't write any test and actually used fmt.Printf should I create an out.Printf or something like this?

Please any suggestions are welcome, I am not really a golang dev =P

To have better options to replace and format, we would need to add more information to the status command.

Comment thread cli/status.go
Copy link
Copy Markdown
Owner

@dominikbraun dominikbraun left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me! I'd merely suggest to directly return in case a format flag has been provided, to better indicate that this flag is a point of return in the program.

Co-authored-by: Dominik Braun <[email protected]>
@dominikbraun dominikbraun self-requested a review June 11, 2021 19:31
@dominikbraun dominikbraun merged commit 3fd9ae3 into dominikbraun:main Jun 12, 2021
@dominikbraun
Copy link
Copy Markdown
Owner

FYI: I briefly renamed two of the formatting variables, see #123.

@sfrique
Copy link
Copy Markdown
Contributor Author

sfrique commented Jun 12, 2021

FYI: I briefly renamed two of the formatting variables, see #123.

Cool, I reviewed the PR.
One thing to note is that I went for the variable at hand and the Formatter func names.
You might want to update the FormatBreakTime and FormatTodayTime as well.

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.

2 participants