Do not warn users if a workflow cannot be read #370
Conversation
I don't think this will send a if (workflow === undefined) {
return [WorkflowErrors.LintFailed];
}Ideally we'd also have different kinds of |
|
I agree this will silence the error message. Up to you if you want to also upload the failure message in the status report or just leave it. |
simon-engledew
force-pushed
the
simon-engledew/hide-workflow-not-found
branch
3 times, most recently
from
6b04c71 to
483255c
Compare
Ugh, sorry - quite right. I should have actually checked this. 🤦 🙇 I've had a bit of a refactor and tweaked the way we report errors to include the exception string if something failed during linting |
robertbrignull
left a comment
robertbrignull
left a comment
There was a problem hiding this comment.
One suggestion about a try-catch but LGTM regardless
src/actions-util.ts
Outdated
| } | ||
| } catch (e) { | ||
| return [WorkflowErrors.LintFailed]; | ||
| return `error: getWorkflow() failed: ${e.toString()}`; |
There was a problem hiding this comment.
To avoid this try-catch covering more than necessary you could do
let workflow: Workflow;
try {
workflow = await getWorkflow();
} catch (e) {
return `error: getWorkflow() failed: ${e.toString()}`;
}and that way you can assign a variable to a non-nullable type but have the try-catch be more contained so it's easier to read and you won't unexpectedly catch something you didn't mean to.
There was a problem hiding this comment.
Oh, it's nice that the type checker is sophisticated enough to realise that the variable is not nullable!
I've used that technique to add more fine grained handling over the whole function, thanks! 👍
simon-engledew
force-pushed
the
simon-engledew/hide-workflow-not-found
branch
from
d947456 to
fed7a74
Compare
This will still send a LintFailed message status report.
simon-engledew
force-pushed
the
simon-engledew/hide-workflow-not-found
branch
from
b3f3e39 to
6be1f5c
Compare
simon-engledew
force-pushed
the
simon-engledew/hide-workflow-not-found
branch
from
3c9fbc6 to
ee4d067
Compare
Hide these warnings:
This will still send a LintFailed message status report.