-
Notifications
You must be signed in to change notification settings - Fork 146
chore(gas): add overview and history docs #3879
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
base: 01-13-fix_otel_enrich_http_traces
Are you sure you want to change the base?
chore(gas): add overview and history docs #3879
Conversation
More templates
@rivetkit/cloudflare-workers
@rivetkit/db
@rivetkit/framework-base
@rivetkit/next-js
@rivetkit/react
rivetkit
@rivetkit/sql-loader
@rivetkit/virtual-websocket
@rivetkit/engine-runner
@rivetkit/engine-runner-protocol
commit: |
PR Review: Add Gasoline Overview and History DocumentationOverall, this is a valuable documentation contribution that helps explain the Gasoline workflow engine. The documentation is well-structured and provides good explanations of complex concepts like workflow history locations and versioning. ✅ Strengths
📝 Suggestions for Improvement1. Terminology Consistency (docs/engine/GASOLINE/OVERVIEW.md:6-10)The description of Activities vs Operations could be clearer about their differences. Both are described as "thin wrappers around native rust functions," which is confusing. Consider elaborating on the distinction:
2. Grammar and Clarity (docs/engine/GASOLINE/OVERVIEW.md:14)Line 14: "rewoken" → "awakened" or "restored" When a workflow is awakened from database back into memory...Also: "to database" → "to the database" (appears multiple times) 3. Location Notation Ambiguity (docs/engine/GASOLINE/WORKFLOW_HISTORY.md:13)The notation - `{0.1}` - the first inserted event before the first event (note: `.` is a separator, not a decimal)4. Missing Context for "Coordinates" vs "Ordinates" (docs/engine/GASOLINE/WORKFLOW_HISTORY.md:7)The distinction between coordinates and ordinates is introduced but could use a clearer explanation. Consider adding: Each __coordinate__ is a set of __ordinates__ which are positive integers. For example, in location `{2, 11, 4}`, there are 3 coordinates, each containing a single ordinate (2, 11, and 4 respectively).5. Example Enhancement (docs/engine/GASOLINE/WORKFLOW_HISTORY.md:67-85)The version check example uses 6. Typo Fix Already Applied ✅Good catch on the typo fix in HIBERNATING_WS.md:25 ("hiberating" → "hibernating"). 7. Comment Clarity (engine/packages/gasoline/src/db/kv/mod.rs:980)The updated comment is slightly clearer but could be more precise: /// Returns the first incomplete workflow with the given name and tags. "First" means the one with the
/// lowest ID value when sorted by the internal byte representation used in the KV store. Due to KV store
/// limitations, only the first matching workflow can be retrieved.
🔍 Technical ConcernsMinor: Documentation vs Implementation DriftThe documentation should include a note about when it was last updated relative to the codebase to help readers understand if the docs might be stale. Consider adding version info or last-updated dates to the doc headers. ✨ Nice Touches
📋 TODO ItemsThe PR creates stubs for:
These are marked as TODO, which is fine for this PR, but consider opening follow-up issues to track completion of these documentation sections. 🎯 SummaryThis is a solid documentation improvement that makes Gasoline more accessible to developers. The main areas for enhancement are:
The code change in Recommendation: Approve with minor suggestions for follow-up improvements. |
| - Separate the transactions into 2 activities | ||
|
|
||
| ## Signals | ||
|
|
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.
Nice how it explains a bit more of how it actually works. Might be good to say blatanlty how signals are the only form of durable/guaranteed communication and they’re consumption is guaranteed ( is it?)
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.
Updated to say "only". Signal consumption is not guaranteed, the wf needs to listen to the signals to receive them
|
|
||
| Signals can be sent to a workflow if you know its name and a subset of its tags. | ||
|
|
||
| Internally, it is more efficient to order signal tags in a manner of most unique to least unique: |
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.
Is this Gotchas worthy?
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.
Definitely
ea05c93 to
c2f846e
Compare
a35f0d9 to
e88b75c
Compare
0738934 to
8b91b37
Compare
e88b75c to
74129c7
Compare
8b91b37 to
f65fde5
Compare
74129c7 to
c3c6740
Compare
f65fde5 to
ffb8072
Compare
c3c6740 to
cdb8231
Compare
ffb8072 to
654b4e9
Compare
cdb8231 to
1353291
Compare
654b4e9 to
c023ef5
Compare
921ccb3 to
e8bbcf8
Compare
c023ef5 to
af232ee
Compare
e8bbcf8 to
d8a0135
Compare
af232ee to
1c80e13
Compare
d8a0135 to
116ea58
Compare
1c80e13 to
5889ab3
Compare

No description provided.