[Workflows] Add workflowName and schedule to WorkflowEvent#6781
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Adds WorkflowCronSchedule type and two new fields (workflowName, schedule) to WorkflowEvent.
Issues (ranked by severity):
-
[LOW]
generated-snapshot/appears hand-edited —types/AGENTS.mdstates: "Do not edit files ingenerated-snapshot/directly — they are overwritten byjust generate-types." Please verify the snapshots were regenerated withjust generate-typesrather than manually edited. CI will reject a mismatch, but running the generator also ensures the blank-line formatting betweenWorkflowStepConfigandWorkflowCronSchedulein the snapshots matches what the pipeline actually produces (thedefines/source has a blank line separator that the snapshots lack). -
[LOW] No type tests for new types —
types/test/types/rpc.tshas tests forWorkflowStepbut none forWorkflowEvent. Adding a basic compile-time check for the new fields (workflowName,schedule,WorkflowCronSchedule) would catch regressions.
This review was generated by an AI assistant and may contain inaccuracies.
|
Review posted successfully on PR #6781. Here's a summary of the findings:
Overall this is a clean, additive change with no backward compatibility concerns. All issues are low severity. |
eefd435 to
5fc6c6d
Compare
Adds two fields to
WorkflowEvent:workflowName: string— name of the workflow the event belongs toschedule?: WorkflowCronSchedule— present when the instance was triggered by a cron scheduleAlso adds the new
WorkflowCronScheduletype (cron,scheduledTime).