Skip to content
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

PubSub routing CloudEvent attribute details and example tweaks #1795

Merged
merged 9 commits into from
Sep 28, 2021

Conversation

pkedy
Copy link
Member

@pkedy pkedy commented Sep 15, 2021

  • Attributes from the CloudEvents spec
  • Examples have withdraw handlers
  • Added a C# SDK example

@pkedy pkedy requested review from a team as code owners September 15, 2021 22:27
Copy link
Contributor

@paulyuk paulyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good Phil. One minor typo and an idea about quickstarts.

For the overview we might consider future rev that first talks about the jobs of routing and filtering based on a match. Then get into there are declarative (more scalable) and code based approaches. We lead off with the declarative guidance before explaining the feature a bit more IMHO.

@pkedy
Copy link
Member Author

pkedy commented Sep 15, 2021

For the overview we might consider future rev that first talks about the jobs of routing and filtering based on a match. Then get into there are declarative (more scalable) and code based approaches. We lead off with the declarative guidance before explaining the feature a bit more IMHO.

Totally agree, @paulyuk. This content was a little rushed TBH.

@orizohar orizohar requested a review from msfussell September 16, 2021 15:42
@orizohar
Copy link

@msfussell can you please review this?

…community call

* Added example CEL expressions
* Added link to 9/21/2021 community call youtube video
* Added section headers for the right nav
@pkedy
Copy link
Member Author

pkedy commented Sep 24, 2021

Note: This depends on a link to dapr/samples#82

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after the small change to the video heading I added.

@pkedy
Copy link
Member Author

pkedy commented Sep 25, 2021

@orizohar would you mind reviewing dapr/samples#82? Once that is merged, the mechanical markdown link checks in this PR can be re-triggered.

@orizohar orizohar merged commit 5ec6217 into dapr:v1.4 Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants