-
Notifications
You must be signed in to change notification settings - Fork 213
Specific interface for provider enhancement (3/3) #1023
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
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.
Holy moly 🥵
That's a thorough API and test suite with a heck of a terse/efficient implementation. Excellent job!
['@type']: 'com.google.firebase.firebasealerts.CrashlyticsRegressionAlertPayload'; | ||
type: string; | ||
issue: Issue; | ||
resolveTime: string; |
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.
FYI: I don't recall all the details, but we have an outstanding bug for the auth namespace that the proto-JSON format isn't the SDK format for date strings. Let's not make the same mistake again.
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.
#1012 for details. Let's bring this up with API council and see if there needs to be an AIP on the standard way going forwards.
); | ||
|
||
const func = (raw: CloudEvent<unknown>) => { | ||
return handler(raw as CrashlyticsEvent<T>); |
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.
Here's where you might need to recognize some date fields and change their format (if API council decides that ISO 8601 isn't the right format). I know you've already gone through review, but it just might be worth bringing up to a few people (e.g. @mbleigh) that we have inconsistencies and need an AIP.
* breaking out general interface * cleaning up exports * removing comments & references to specific interfaces * format * jsdoc comments * address pr comments * added param comment * addressing comments * Specific interface for provider enhancement (1/3) (#1021) * adding in app distro changes * removing comments & addings package refs * jsdoc comments * fix comments * adding periods to doc strings * fix wording * adding import/export statement * Specific interface for provider enhancement (2/3) (#1022) * adding in billing changes * removing comments & adding package refs * jsdoc comments * addressing pr comments * change handler doc string * changing to BillingEventHandler type * remove BillingEventHandler type * Specific interface for provider enhancement (3/3) (#1023) * adding in crashlytics changes * comments & adding package refs * address comments and make doc strings better * add opts.retry to event trigger
* breaking out general interface * cleaning up exports * removing comments & references to specific interfaces * format * jsdoc comments * address pr comments * added param comment * addressing comments * Specific interface for provider enhancement (1/3) (#1021) * adding in app distro changes * removing comments & addings package refs * jsdoc comments * fix comments * adding periods to doc strings * fix wording * adding import/export statement * Specific interface for provider enhancement (2/3) (#1022) * adding in billing changes * removing comments & adding package refs * jsdoc comments * addressing pr comments * change handler doc string * changing to BillingEventHandler type * remove BillingEventHandler type * Specific interface for provider enhancement (3/3) (#1023) * adding in crashlytics changes * comments & adding package refs * address comments and make doc strings better * add opts.retry to event trigger
Change for the the specific crashlytics interface. Exposes
onNewFatalIssuePublished
,onNewNonfatalIssuePublished
,onRegressionAlertPublished
,onStabilityDigestPublished
,onVelocityAlertPublished
, andonNewAnrIssuePublished
from thev2/alerts/crashlytics
namespaceEx ~