Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
[Workflow] Add tip for workflow configuration default values#7315
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
javiereguiluz left a comment
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.
Adding this tip looks like a good idea to me. Sadly, it's hard to understand it, so I'd recommend to reword it a bit. Thanks!
dominikhajduk commentedJan 8, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@javiereguiluz thanks for CR. I've rewritten tip to be clearer. If you have additional suggestions please let me know. |
javiereguiluz commentedJan 8, 2017
@dominikhajduk thanks for this reword. I can now understand everything perfectly 👍 |
javiereguiluz left a comment
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.
👍
workflow/usage.rst Outdated
| Workflow configuration ``type`` and ``arguments`` in ``marking_store`` section | ||
| are optional. You can skip one or both when you use default values. Default value | ||
| for ``type`` is "single_state". Default value for ``arguments`` is "marking". |
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.
I would reword this a bit to something like this:
The
type(default valuesingle_state) andarguments(default valuemarking) attributes of themarking_storeoption are optional. If omitted, their default values will be used.
What do you think?
dominikhajdukJan 10, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
@xabbuh fine for me. Will change in couple of seconds.
dominikhajduk commentedJan 10, 2017
@xabbuh should be fine now. Thanks |
xabbuh commentedJan 10, 2017
👍 @javiereguiluz Good for you too? |
javiereguiluz commentedJan 10, 2017
@xabbuh yes, it looks very nice! |
xabbuh commentedJan 11, 2017
Thank you Dominik. |
…ues (dominikhajduk)This PR was squashed before being merged into the 3.2 branch (closes#7315).Discussion----------[Workflow] Add tip for workflow configuration default valuesAdded tip forhttp://symfony.com/doc/current/workflow/usage.html configuration.Tested and based on great examples provided by@lyrixx in:https://github.com/lyrixx/SFLive-Paris2016-Workflow/blob/master/app/config/workflow.yml#L3 andhttps://github.com/lyrixx/SFLive-Paris2016-Workflow/blob/master/src/AppBundle/Entity/Article.php#L24Commits-------6262d24 [Workflow] Add tip for workflow configuration default values
Added tip forhttp://symfony.com/doc/current/workflow/usage.html configuration.
Tested and based on great examples provided by@lyrixx in:https://github.com/lyrixx/SFLive-Paris2016-Workflow/blob/master/app/config/workflow.yml#L3 and
https://github.com/lyrixx/SFLive-Paris2016-Workflow/blob/master/src/AppBundle/Entity/Article.php#L24