doctor: add error messages for failures #672
Conversation
jmeistrich
requested review from
Esemesek,
grabbou and
thymikee
as code owners
|
@jmeistrich can I ask you to rebase? We've added types to doctor command and now conflicts arisen |
thymikee
changed the title
jmeistrich
force-pushed
the
doctor/errorMessages
branch
from
7377a45 to
95d863e
Compare
|
CI is failing because of known race conditions (with yet unknown cause) in config test |
|
Thank you! |
|
Happy to help! |
| }) => { | ||
| const symbol = needsToBeFixed | ||
| ? isRequired | ||
| ? chalk.red('✖') | ||
| : chalk.yellow('●') | ||
| : chalk.green('✓'); | ||
|
|
||
| logger.log(` ${symbol} ${label}`); | ||
| logger.log(` ${symbol} ${label}${needsToBeFixed ? ': ' + description : ''}`); |
There was a problem hiding this comment.
There was a problem hiding this comment.
That's the reason – to show why the check failed. We treat it as first pass, feel free to iterate on how to present it better :)
There was a problem hiding this comment.
Perhaps it would be nice to show this only with --verbose?
There was a problem hiding this comment.
Nah, why hiding the reason behind failure? Running doctor again may take some time as well.
There was a problem hiding this comment.
Good point.
I would like to, at least, get rid of should be installed as there isn't much being provided there, thoughts?
There was a problem hiding this comment.
You mean that each health check file can provide its own description?
There was a problem hiding this comment.
Something like this could be added to getDiagnostics() output. Currently needsToBeFixed can be a boolean or string. But we could make it a bit more explicit and add extra information like error message/descirption. Or we can keep the current behavior and only do something like:
getDiagnostics: async () => ({
needsToBeFixed: await checkSoftwareInstalled('pod') || "custom message",
}),There was a problem hiding this comment.
If I got what you mean right, it would be nice to do:
getDiagnostics: async () => ({
description: 'required for installing dependencies',
needsToBeFixed: await checkSoftwareInstalled('pod'),
}),Is that it?
Summary:
This adds error messages for failing checks.