feat(*): sync examples to v0.8 spec #100
Conversation
|
model/workflow.go
Outdated
|
|
||
| // TODO: optimize this | ||
| // In the specification, we can declared independently definitions with another file format, so | ||
| // we must convert the independently yaml source to json before unmarshal. |
There was a problem hiding this comment.
@lsytj0413 wdyt about opening an issue for this one?
There was a problem hiding this comment.
nice advice, i will open an issue and fix it in another PR.
| "operation": "file://myapis/emailapis.json#paymentInsufficientFunds" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Can't we move these files to testdata directory?
There was a problem hiding this comment.
In the paymentconfirmation.json file, workflow definition refs functionsdefs.json as functiondefs.json; so if we change move this files to testdata, we must:
- modify the example definitions refs to another value, this will make sync the examples from specification difficulty. (some example's definitions must be modify, because it refs other file with abs path, ex
file://books/lending/functions.json) - change the root path of not absolute file to testdata directory (currently it's
parserdirectory)
wdyt we can do?
There was a problem hiding this comment.
I think "2" could be a better option then because having these files lying in the package smells. At least in the testdata directory, we add purpose.
model/states.go
Outdated
| @@ -139,8 +140,8 @@ type OperationState struct { | |||
|
|
|||
| // OperationStateTimeout ... | |||
| type OperationStateTimeout struct { | |||
| StateExecTimeout StateExecTimeout `json:"stateExecTimeout,omitempty"` | |||
| ActionExecTimeout string `json:"actionExecTimeout,omitempty" validate:"omitempty,min=1"` | |||
| StateExecTimeout *StateExecTimeout `json:"stateExecTimeout,omitempty"` | |||
There was a problem hiding this comment.
maybe you can revert this change as the timeouts will be fixed as part of #104
|
small fixes for #70 |
|
@lsytj0413 can you please fix the conflicts? |
#105 should be merge first, i will resolve this conflict after 105 have merged. |
It was merged, but we still have one file to solve the conflict. |
Signed-off-by: lsytj0413 <511121939@qq.com>
Fixed，PTAL. There are following actions:
I will fix this in another PR. |
| if err := json.Unmarshal(eventBasedSwitch["timeouts"], &j.Timeouts); err != nil { | ||
| return err | ||
| eventBaseTimeoutsRawMessage, ok := eventBasedSwitch["timeouts"] | ||
| if ok { |
There was a problem hiding this comment.
I think it is not needed, right?
There was a problem hiding this comment.
We must have this because the timeouts field is optional. If we don't have it, checkcarvitals.json and eventbasedswitchstate.json will failed with unexpected end of JSON input because we try to unmarshal empty string.
Signed-off-by: lsytj0413 511121939@qq.com
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Special notes for reviewers:
Additional information (if needed):