Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Reduce potential confusion in priority explanation#10655
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
I always get tripped up by event priority. In my mind, I think priority 1 executes before priority 2. I'm backwards, I know...This PR should help remove any confusion for myself and others by explicitly stating "priority 3 executes before priority 2" and so on.
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.
Reasonable change, people can get easily confused.
components/event_dispatcher.rst Outdated
| #. An optional priority integer that determines when a listener is triggered | ||
| versus other listeners (defaults to ``0``). A higher priority integer means | ||
| the listener will be triggered earlier. So, priority 3 executes before | ||
| priority 2. Priority 2 executes before priority 1. Priority 1 executes before |
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.
worth hinting negative numbers are supported?
Shaun, thanks a lot for helping us improve this doc. We reworded your proposal a bit to make it more concise. The reason is that this priority concept is explained tens of times in the Symfony docs, so we know from experience that longer and fully detailed explanations tend to confused readers rather than help them. We reworded it a bit to make it similar to the other priority explanations found in the docs. Thanks! |
…haun, javiereguiluz)This PR was merged into the 2.8 branch.Discussion----------Reduce potential confusion in priority explanationI always get tripped up by event priority. In my mind, I think priority 1 is higher than priority 2. I guess I'm backwards :)This PR should help remove any confusion for myself and others by explicitly stating "priority 3 executes before priority 2" and so on.Commits-------afc8c31 Reworded4da8de0 Reduce potential confusion in priority explanation
That defeats the purpose of why I made this PR, and I'm not sure how being explicit about how it works makes it more confusing for readers. My goal was to define exactly what "higher" meant in this case. |
The key is this phrase, which we think is enough to understand it:
We use the same explanation ("the higher the priority, the earlier ...") in many other parts of the Symfony Docs ... and that's why we think it's better to make it consistent with those other explanations of the same feature. |
I understand the need for consistency, but it's that phrase in particular that I wanted to clarify. At least to me, "higher" is ambiguous in the context of a "priority" number. Logically, I can think of it as "1st priority, 2nd priority, 3rd priority", which is the opposite of Symfony; or, I can think of it as "greater numbers execute first". It's because of that, I get tripped up occasionally when I haven't had to mess with event priorities in a long time. |
what about using another word than "priority", eg: |
I understand the problem now and I think you are completely right. I was so used to this explanation ... that I was blind. What about Nico's proposal? I think it would solve the problem. |
I agree; that tells me exactly how it functions. Should I make a new PR? Not sure of the best (or most convenient for you) way to handle this since this one was merged. |
Sure thing. Only one sentence read a little awkwardly to me. Other than that, lgtme. Thank you! |
Uh oh!
There was an error while loading.Please reload this page.
I always get tripped up by event priority. In my mind, I think priority 1 is higher than priority 2. I guess I'm backwards :)
This PR should help remove any confusion for myself and others by explicitly stating "priority 3 executes before priority 2" and so on.