8000 Add basic format option by sfrique · Pull Request #114 · dominikbraun/timetrace · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add basic format option #114

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 12, 2021
Merged

Add basic format option #114

merged 2 commits into from
Jun 12, 2021

Conversation

sfrique
Copy link
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.

dominikbraun
dominikbraun reviewed Jun 11, 2021
View reviewed changes
dominikbraun
dominikbraun reviewed Jun 11, 2021
View reviewed changes
Copy link
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.

@sfrique @dominikbraun
Update cli/status.go
aba2f72 
Co-authored-by: Dominik Braun <mail@dominikbraun.io>
@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
Owner

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

@sfrique
Copy link
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.

@sfrique sfrique mentioned this pull request Jun 12, 2021
Closed
@dominikbraun dominikbraun mentioned this pull request Jun 23, 2021
Closed
@dominikbraun dominikbraun mentioned this pull request Sep 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominikbraun dominikbraun Awaiting requested review from dominikbraun

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sfrique @dominikbraun
0