-
-
Notifications
You must be signed in to change notification settings - Fork 325
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
Fix #3571: Added detailed feedback for successful task synchronization #3758
base: develop
Are you sure you want to change the base?
Fix #3571: Added detailed feedback for successful task synchronization #3758
Conversation
… task synchronization
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!
Some users embed task sync
in various automation (cron tasks, shell scripts, etc.), and those users may be unhappy to see output unconditionally appear. Please make these new messages appear only when verbose output is enabled, just like the pre-sync messages.
Also, the pre-sync messages include the type of sync, so it seems redundant to repeat that afterward.
Finally, it'd be nice to give the user some additional information about what happened. TaskChampion doesn't expose the number of new operations it received from the server, but the replica method num_local_operations
does give a hint as to what will be sent to the server (and after a successful sync, can be assumed to have been sent). So, let's include that value in the output.
@@ -159,6 +162,11 @@ int CmdSync::execute(std::string& output) { | |||
context.tdb2.expire_tasks(); | |||
} | |||
|
|||
if (verbose) { | |||
out << format("Successfully synchronized {1} operations", replica->num_local_operations()) |
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.
This will always be zero, right?
@antek5421 will you have a chance to finish this up? |
This pull request includes changes to the
CmdSync::execute
method insrc/commands/CmdSync.cpp
to improve logging by adding messages that confirm successful synchronization with different targets.Logging improvements:
Fixes #3571