Implemented eventarc event publishing API #1617
Conversation
egilmorez
left a comment
egilmorez
left a comment
There was a problem hiding this comment.
Some style things to look at in the comments. Thanks!
| @@ -0,0 +1,25 @@ | |||
|
|
|||
| /** | |||
| * A CloudEvent version. | |||
There was a problem hiding this comment.
Should this be backticked just like Eventarc in firebase-namespace.ts?
Or is CloudEvent just a camel-cased product name?
There was a problem hiding this comment.
yeah, I think CloudEvent here is, not a product name, but the spec... don't think it needs the backticks.
lahirumaramba
left a comment
lahirumaramba
left a comment
There was a problem hiding this comment.
Thank you @pavelgj !
Didn't dive into tests, yet. Added a few comments, mostly style changes and suggestions.
lahirumaramba
left a comment
lahirumaramba
left a comment
There was a problem hiding this comment.
Thank you for addressing the PR feedback @pavelgj !
Looks pretty good! Left a few more comments.
lahirumaramba
left a comment
lahirumaramba
left a comment
There was a problem hiding this comment.
Thank you @pavelgj. LGTM!
Do we plan to add any integration tests later?
|
Yes, I will work on the integration tests. |
3 participants
go/gnqghm1gqpp6