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

update log helptext to match actual levels #7199

Merged
merged 2 commits into from Apr 24, 2020

Conversation

djdv
Copy link
Contributor

@djdv djdv commented Apr 23, 2020

It looks like "warning" and "critical" levels were removed, but the help text wasn't updated to reflect this.

> ipfs.exe version --all
go-ipfs version: 0.5.0-rc3
Repo version: 9
System version: amd64/windows
Golang version: go1.13.10
> ipfs.exe log level all warning
Error: unrecognized level: "warning"
> ipfs.exe log level all critical
Error: unrecognized level: "critical"

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Thanks for catching this! "warning" was renamed to "warn".

core/commands/log.go Outdated Show resolved Hide resolved
core/commands/log.go Outdated Show resolved Hide resolved
@djdv
Copy link
Contributor Author

djdv commented Apr 23, 2020

I noticed the other values like DPANIC within the log package are accepted via this command too. Should they be added?

// Supported strings are: DEBUG, INFO, WARN, ERROR, DPANIC, PANIC, FATAL, and
// their lower-case forms.

https://github.com/ipfs/go-log/blob/54443fbc2ea88e1a8262b668cd948d3f1247cd1b/levels.go#L22-L23

@Stebalien
Copy link
Member

Ideally, yes.

@djdv
Copy link
Contributor Author

djdv commented Apr 24, 2020

Should be all good now.
(I did notice some of the helptext keeps to the 80 character bound, but not all of it. So I'm guessing this is fine without an explicit line-break)

@Stebalien
Copy link
Member

I'd try to keep to 80 characters, we don't wrap elsewhere.

Could you rebase on master (and squash)? That should fix CI.

@djdv
Copy link
Contributor Author

djdv commented Apr 24, 2020

I put the possible values for both IPFS_LOGGING and IPFS_LOGGING_FMT on a newline that's indented under them. Both fall under 80 and look consistent. If they should look another way just let me know.

This is how it's displaying for me:
image

@Stebalien Stebalien merged commit b19d57f into ipfs:master Apr 24, 2020
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.

None yet

2 participants