-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[#32601][prism] Initial Deep Dive Documentation #32143
Conversation
8980099
to
15b7bf7
Compare
15b7bf7
to
3c5d8c6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #32143 +/- ##
============================================
- Coverage 59.11% 59.11% -0.01%
Complexity 4499 4499
============================================
Files 1316 1316
Lines 180967 181233 +266
Branches 4361 4361
============================================
+ Hits 106981 107134 +153
- Misses 70289 70388 +99
- Partials 3697 3711 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
19e1a22
to
161f625
Compare
Assigning reviewers. If you would like to opt out of this review, comment R: @jrmccluskey for label go. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
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.
First pass of review edits
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 for the first pass!
I'm definitely on the side of Good Enough vs Perfect when it comes to this documentation, and then subsequent changes can be more focused. May I have an estimate on when another pass will occur? |
I will do a quick second pass first thing tomorrow morning, should be sufficient for an approval at that point |
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.
LGTM mod the remaining typos
Co-authored-by: Jack McCluskey <34928439+jrmccluskey@users.noreply.github.com>
Add additional, initial, in depth documentation for how Prism executes pipelines, and it's various systems.
See the rendered view for the mermaid diagrams. https://github.com/apache/beam/pull/32143/files?short_path=15906a7#diff-15906a7fc80b5c1ff20d96ba3d8348452e1be358d5ba7e187c3d75776fb1b9f6
Fixes #32601
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.