-
Notifications
You must be signed in to change notification settings - Fork 310
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
Treat status reports as non-critical #2121
Treat status reports as non-critical #2121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this — our telemetry reporting should indeed be non-critical! I've just added a few comments.
I think this may warrant a CHANGELOG entry 🌞 users who may have seen this failure in the past should be happy to see the change.
6145d06
to
d3f9862
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. I have a few copy-editing suggestions and one question about 404/403 errors.
ac030dd
to
5942183
Compare
@@ -16,6 +16,11 @@ import { parseRepositoryNwo, RepositoryNwo } from "./repository"; | |||
import * as util from "./util"; | |||
import { SarifFile, ConfigurationError, wrapError } from "./util"; | |||
|
|||
const GENERIC_403_MSG = | |||
"The repo on which this action is running has not opted-in to CodeQL code scanning."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message and the one below it needs some work, but as you mentioned, it is outside the scope of this PR.
Thanks again for your work on this. |
Change `sendStatusReport` to `void`
df25046
to
5a6da1d
Compare
Thanks again! Nice work here. |
Resolve github/codeql#15462 (comment) by converting
sendStatusReport
into a function that always returnsvoid
and usescore.warning
instead ofcore.error
Split from:
Merge / deployment checklist