-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Clarify the "enter(-ed)" events #9210
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
Conversation
@lyrixx since you are our biggest Workflow expert, could you please review these proposed changes? Thanks! |
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.
Thanks for the clarification.
I let few minor comments
workflow/usage.rst
Outdated
is marked as being in the new place. | ||
The object is about to enter a new place. This is the event where the object is | ||
going to be marked as being in the new place. | ||
Please notice: the marker store of the object is not yet updated with the new state. |
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 catch. But the vocabulary is not accurate.
Please notice : the marking of the object is not ...
The marking
is the representation of the subject in the workflow
The marking store
is the interface between the subject and the workflow
The marker store
does not exist
s/state/places/ (plural!)
workflow/usage.rst
Outdated
@@ -265,8 +266,8 @@ order: | |||
* ``workflow.[workflow name].enter.[place name]`` | |||
|
|||
``workflow.entered`` | |||
Similar to ``workflow.enter``, except the marking store is updated before this | |||
event (making it a good place to flush data in Doctrine). | |||
The object has entered a state and the marker store is updated (making it a good |
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.
has entered in the places
(I'm not so fluent, but we should talk aboutplaces
(plural) and notstate
)and the marking is updated
(not themarking store
)
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.
Fixed!
workflow/usage.rst
Outdated
The object entered a new place. This is the first event where the object | ||
is marked as being in the new place. | ||
The object is about to enter a new place. This is the event where the object is | ||
going to be marked as being in the new place. |
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.
Actually, this is the event before the subject (everywhere in the code I used subject and not object, I think it's better if the doc reflects that) is going to be updated
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 as:
The subject is about to enter a new place. This is the event triggered before the subject is
going to be updated as being in the new places.
Please notice: the marking of the subject is not yet updated with the new places.
Thanks @javiereguiluz and @lyrixx for the review! |
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.
👍 for the technical part.
I let others review the style ;)
Thanks for this PR.
Note: The target branch (4.0) is wrong, it should be 3.3
Thanks @lyrixx I changed the target branch and rebased my changes on the |
@thePanz thanks for this contribution and for your patience during the review process. Regarding the right branch for this change, it should be 3.4 because 3.3 is no longer maintained. For future contributions, don't worry much about the branch because doc maintainers can change it when merging. Thanks! |
This PR was submitted for the 3.3 branch but it was merged into the 3.4 branch instead (closes #9210). Discussion ---------- Clarify the "enter(-ed)" events Added clarification about the marker-store for the `enter` and `entered` events. Commits ------- f73660d Minor reword 92abab6 Clarify the "enter(-ed)" events
Thanks @javiereguiluz ! :) |
Added clarification about the marker-store for the
enter
andentered
events.