Conversation
|
How do I update the manual at readthedocs.io? |
|
The source for the documentation is in https://github.com/restic/restic/tree/master/doc . ReadTheDocs is automatically updated with the |
MichaelEischer
left a comment
There was a problem hiding this comment.
Sorry to be blunt. The PR completely ignores all error messages (see all Printf statements). As a consequence the JSON output would break once the check command actually detects an error, thus making the current changes pointless. This is worse than the current state of no JSON output at all.
Thus either convert everything (we can discuss how to handle stderr) or drop the PR.
| bar := newProgressMax(!gopts.Quiet, 0, "snapshots") | ||
| defer bar.Done() | ||
| if !gopts.JSON { | ||
| bar := newProgressMax(!gopts.Quiet, 0, "snapshots") |
There was a problem hiding this comment.
With this change, chkr.Structure uses the wrong instance of the bar variable.
| status := checkSuccess{ | ||
| MessageType: "checked", | ||
| Message: "no errors were found", | ||
| ID: repo.Config().ID, |
There was a problem hiding this comment.
The ID and Repository URL aren't really relevant here and thus should be removed.
|
Thanks for working on this! I think a single summary output in the end might be enough for many use cases (including mine of running restic from a script). Although one should reliably receive (at least) one json message (in both success and failure cases). |
It's far simpler to just inspect the exit code of the |
|
Closing as this PR needs a full rework anyways, feel free to open a new one if you're interested in addressing the review feedback. |
What does this PR change? What problem does it solve?
Adding json logging to the check command, in a similar fashion how the init command outputs json.
Was the change previously discussed in an issue or on the forum?
No
Checklist
changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.