Skip to content

Conversation

@tsurdilo
Copy link
Contributor

Signed-off-by: Tihomir Surdilovic tihomir@temporal.io

Many thanks for submitting your Pull Request ❤️!

Please specify parts this PR updates:

  • [ x] Specification
  • Schema
  • Examples
  • Extensions
  • Roadmap
  • Use Cases
  • Community
  • TCK
  • Other

What this PR does / why we need it:
adds clarification on some mutually exclusive properties
Special notes for reviewers:

Additional information (if needed):

@tsurdilo tsurdilo added the change: documentation Improvements or additions to documentation. It won't impact a version change. label Dec 23, 2021
@tsurdilo tsurdilo added this to the v0.9 milestone Dec 23, 2021
@tsurdilo tsurdilo linked an issue Dec 23, 2021 that may be closed by this pull request
specification.md Outdated
| [compensatedBy](#Workflow-Compensation) | Uniaue name of a workflow state which is responsible for compensation of this state | String | no |
| [usedForCompensation](#Workflow-Compensation) | If true, this state is used to compensate another state. Default is "false" | boolean | no |
| [metadata](#Workflow-Metadata) | Metadata information| object | no |
| [metadata](#Workflow-Metadata) | Metadata information| object | yes if "transition" is not defined |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

specification.md Outdated
Note that the `transition` and `end` properties are mutually exclusive, meaning that you can only specify one or the other,
but not both at the same time.

Note that `transition` and `end` properties are mutually exclusive, meaning that you cannot define both of them at the same time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn’t this duplicate of the line 4155

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

| --- | --- | --- | --- |
| errorRef | Reference to a unique workflow error definition | string | yes if errorRefs is not used |
| errorRefs | Reference one or more unique workflow error definition | array | yes if errorRef is not used |
| errorRef or errorRefs | Reference one unique workflow error definition, or multiple unique workflow error definitions | string (errorRef) or array (errorRefs) | yes |
Copy link

@DoisKoh DoisKoh Dec 23, 2021

Choose a reason for hiding this comment

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

Are errorRef and errorRefs both allowed to be specified in a single Error Definition? Also, is the actually "ErrorRef Definition" rather than "Error Definition"?

Copy link
Member

Choose a reason for hiding this comment

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

If I understood your question correctly, no. And below we have the explanation:

Note that the `errorRef` and `errorRefs` properties are mutually exclusive, meaning that you can only specify one or the other, but not both at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DoisKoh

Also, is the actually "ErrorRef Definition" rather than "Error Definition"?

not sure i understand, these props are part of the "Error" definition - https://github.com/serverlessworkflow/specification/blob/main/schema/workflow.json#L327

maybe I am missing something

Copy link

Choose a reason for hiding this comment

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

Ah right, well it's a bit confusing because... there's separate "ErrorDef" definition, thought it might be more apt to have different names

https://github.com/serverlessworkflow/specification/blob/main/schema/errors.json

@tsurdilo tsurdilo force-pushed the clarifyexclusiveprops branch from 85e2774 to a2b943a Compare December 24, 2021 03:11
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
@tsurdilo tsurdilo force-pushed the clarifyexclusiveprops branch from a2b943a to 6f81c32 Compare December 25, 2021 06:39
@tsurdilo tsurdilo merged commit bbad386 into serverlessworkflow:main Dec 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change: documentation Improvements or additions to documentation. It won't impact a version change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clarity on exclusive options in specs

4 participants