Diagnostics: path management #7297
Conversation
This makes it possible for a diagnostics checker to return multiple results; this will be used for the path management diagnostics. Signed-off-by: Mark Yen <[email protected]>
This actually just stores the result from the last path management run in memory, and report that when requested. Signed-off-by: Mark Yen <[email protected]>
Update `manageLinesInFile` to throw unique errors so that the diagnostics has more information with which to provide better fix suggestions. Signed-off-by: Mark Yen <[email protected]>
Signed-off-by: Mark Yen <[email protected]>
The `kubeConfigSymlink` checker only makes sense on Windows. Signed-off-by: Mark Yen <[email protected]>
| @@ -7,9 +7,15 @@ | |||
| export interface PathManager { | |||
| /** The PathManagementStrategy that corresponds to the implementation. */ | |||
| readonly strategy: PathManagementStrategy | |||
| /** Makes real any changes to the system. Should be idempotent. */ | |||
| /** | |||
| * Makes real any changes to the system. Should be idempotent, and should not | |||
There was a problem hiding this comment.
Is this meant to say "Makes real any changes to the system"?
There was a problem hiding this comment.
Probably? I just added a new line here (and added stuff to the second sentence).
I'll reword this to Applies changes to the system instead.
| @@ -47,6 +47,7 @@ export class DiagnosticsManager { | |||
| this.checkers = diagnostics ? Promise.resolve(diagnostics) : (async() => { | |||
| const imports = (await Promise.all([ | |||
| import('./connectedToInternet'), | |||
| import('./pathManagement'), | |||
There was a problem hiding this comment.
Does the order matter here? If not, I sorted them out in alphabetical order.
There was a problem hiding this comment.
Ah, sorry about that, I'll make it alphabetical.
|
|
||
| this.results[checker.id] = result; | ||
| if (Array.isArray(result)) { | ||
| for (const singleResult of result) { |
There was a problem hiding this comment.
Yes; it's not like there will be that many elements.
| const console = Logging.diagnostics; | ||
| const cachedResults: Record<string, DiagnosticsCheckerResult> = {}; | ||
|
|
||
| const CheckPathManagement: DiagnosticsChecker = { |
There was a problem hiding this comment.
Does this need some documentation like other diagnostics?
|
|
||
| mainEvents.on('diagnostics-event', (id, state) => { | ||
| console.log('diagnostics-event', id, state); | ||
| if (id !== 'path-management') { |
There was a problem hiding this comment.
nit; I noticed that path-management id is scattered all over the place, even in all the calls (around 4 calls) to mainEvents.emit('diagnostics-event', 'path-management', { fileName, error });.
There was a problem hiding this comment.
I'll make the mainEvents.emit() call typed so that a typo is a compile-time error.
Also drops one line of excess logging. Signed-off-by: Mark Yen <[email protected]>
This implements diagnostics for path management so people can see the errors that occur.
This also adds the ability for diagnostics to return multiple results, because there are too many files under management to list just one.
Fixes #7262