Skip to content
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

Init command JSON output #3132

Merged
merged 4 commits into from Dec 2, 2022
Merged

Conversation

metalsp0rk
Copy link
Contributor

What does this PR change? What problem does it solve?

Enablement of JSON output on the init command.

Was the change discussed in an issue or in the forum before?

Fixes #3124

Checklist

  • I have read the Contribution Guidelines
  • I have enabled maintainer edits for this PR
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

@metalsp0rk metalsp0rk force-pushed the init-json branch 2 times, most recently from ffe5ed8 to 53f94b1 Compare November 28, 2020 21:38
Copy link
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

I've added a few minor comments to the PR. However, there's one point which requires discussion: All other places in restic just return the standard error message when an error happens and don't encode those as JSON. It might for now be better to stay consistent with the other commands.

cmd/restic/cmd_init.go Outdated Show resolved Hide resolved
cmd/restic/cmd_init.go Outdated Show resolved Hide resolved
cmd/restic/cmd_init.go Outdated Show resolved Hide resolved
cmd/restic/cmd_init.go Outdated Show resolved Hide resolved
changelog/unreleased/issue-3124 Show resolved Hide resolved
cmd/restic/cmd_init.go Outdated Show resolved Hide resolved
@metalsp0rk
Copy link
Contributor Author

there's one point which requires discussion: All other places in restic just return the standard error message when an error happens and don't encode those as JSON. It might for now be better to stay consistent with the other commands.

So my rationale here is that it would benefit us to output errors as a JSON object because if a wrapper script is expecting JSON output, the script will not be able to easily handle non-JSON output errors. Of course, it could just panic and alert the user, but in some cases, issues that cause failure could be automatically solved by a wrapper script.

I'm okay with staying consistent with other commands for now. Maybe roll those error handling changes into another PR if we decide we want that, down the line.

@metalsp0rk metalsp0rk force-pushed the init-json branch 2 times, most recently from ce70a54 to 5ecddd1 Compare September 7, 2021 19:39
@metalsp0rk
Copy link
Contributor Author

I'm ready to merge this. Can I get another review on this?

@MichaelEischer
Copy link
Member

I'm ready to merge this. Can I get another review on this?

I totally forgot about this PR. It's now rebased and slightly cleaned up. The status field is now called message_type as that is also used to identify the message type in other places.

The remaining part is to decide on a message_type name. Right now, it is repository_created, although something shorter like summary might also be suitable.

Copy link
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM.

@MichaelEischer MichaelEischer merged commit 0af89a5 into restic:master Dec 2, 2022
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.

restic init command no json output
2 participants