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

Feat/better inspect output 2720 #2730

Merged
merged 3 commits into from
May 10, 2019
Merged

Conversation

rosalinekarr
Copy link
Contributor

Problem

The output from the inspect commands are difficult to read by default.

Solution

Use pretty-printed json for text output, and maintain compact json for use with the --enc=json flag.

Resolves #2720

@rosalinekarr rosalinekarr self-assigned this May 6, 2019
@rosalinekarr rosalinekarr force-pushed the feat/better-inspect-output-2720 branch 4 times, most recently from db441db to bc1f2ab Compare May 9, 2019 13:13
@frrist
Copy link
Member

frrist commented May 9, 2019

I don't think the text encoder should be outputting json. I would prefer it output a table, or even just plain text. Here are some examples of what I am thinking:

$ go-filecoin inspect disk
Disk:
| key    | value       |
|--------|-------------|
| FSType | 61267       |
| Free   | 856702431232|
| Total  | 805490962432|

or

$ go-filecoin inspect disk
Disk
FSType: 61267
Free:   856702431232
Total:  805490962432

If a user wants the output of a command to be pretty printed they can achieve this using jq, e.g.

$ go-filecoin inspect disk | jq
{
  "FSType": "61267",
  "Free": 856702431232,
  "Total": 805490962432
}

Copy link
Member

@frrist frrist left a comment

Choose a reason for hiding this comment

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

I would prefer the changes I have described in my previous comment.

@rosalinekarr rosalinekarr force-pushed the feat/better-inspect-output-2720 branch from bc1f2ab to 403609d Compare May 9, 2019 18:55
@rosalinekarr rosalinekarr requested a review from frrist May 9, 2019 18:56
@rosalinekarr
Copy link
Contributor Author

@frrist Comments addressed

Copy link
Member

@frrist frrist left a comment

Choose a reason for hiding this comment

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

one blocking comment, then lgtm

commands/inspector.go Show resolved Hide resolved
commands/inspector.go Show resolved Hide resolved
@rosalinekarr rosalinekarr force-pushed the feat/better-inspect-output-2720 branch 2 times, most recently from 08a7645 to 0ab24fc Compare May 10, 2019 13:18
@rosalinekarr rosalinekarr force-pushed the feat/better-inspect-output-2720 branch from 0ab24fc to 867bcc9 Compare May 10, 2019 17:01
Copy link
Contributor

@acruikshank acruikshank left a comment

Choose a reason for hiding this comment

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

LGTM

@rosalinekarr rosalinekarr merged commit dcb0be3 into master May 10, 2019
@rosalinekarr rosalinekarr deleted the feat/better-inspect-output-2720 branch May 10, 2019 18:17
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.

Improve text output from go-filecoin inspect command
3 participants