-
Notifications
You must be signed in to change notification settings - Fork 76
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
#38 revert flag #93
#38 revert flag #93
Conversation
… rename the project filename
@joshuaherrera Feel free to ping me once this is ready to review! |
@dominikbraun This is ready, I won't make anymore changes, unless requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, I like your implementation! It looks pretty good to me, I only requested an extremely nit-picky change to emphasize that the --revert
flag always leads to an return
without any other logic being executed. My other questions regard runtime performance.
Co-authored-by: Dominik Braun <mail@dominikbraun.io>
Co-authored-by: Dominik Braun <mail@dominikbraun.io>
Co-authored-by: Dominik Braun <mail@dominikbraun.io>
Co-authored-by: Dominik Braun <mail@dominikbraun.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for implementing this powerful feature! 👍 This will be the main feature of the next minor release.
This PR is contained in release 0.8.0. |
It was a pleasure! I really enjoy the project layout you set forth in timetrace, it makes coding quite enjoyable! |
This pull request resolves #38 .
Currently, I rely on the user input record / project key to fetch the correct record, since relying on the struct's Key field can cause the record / project to not be found as reported in issues #81 and #89 , though I don't think this will need to change when those issues are resolved.