Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Fixed event listeners priority#3719
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
tony-co commentedMar 25, 2014
| Q | A |
|---|---|
| Doc fix? | yes |
| New docs? | no |
| Applies to | all |
| Fixed tickets | - |
| from -255 to 255, and the warmers will be executed in the order of their | ||
| priority. | ||
| The ``priority`` value is optional, and defaults to 0. The core Symfony | ||
| listeners priorities varies from -1024 to 1024, and the warmers will be |
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.
"vary" because it's listener priorities, no?
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.
indeed
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.
and the comma before "and" should be removed
👍 |
For the next time you submit a fix: If you answer "all" to "Applies to", you should base your PR on the 2.3 branch (since it's the oldest still maintained version). If you answer with a version to "Applies to", it should be based on the branch representing that version (where |
@wouterj Ok got it. |
Actually, this is not true. The core cache warmers are: One has a priority of The previous sentence was also not true, there are no limitations of the priority. |
Indeed. |
Cache warmers are no listeners. However, I think we should remove "The core Symfony listeners priorities vary from -1024 to 1024 and the warmers will be executed in the order of their priority." completely. It doesn't make sense and is not really usefull... However, we can add a table like for the listeners showing the 3 core cache warmers and their priority. |
We need a sentence that explain "The bigger the priority, the sooner is the execution". |
Okay, I removed the phrasing and put a quick note about priority along with a table of the three cache warmers. |
| The higher the priority, the sooner it gets executed. | ||
| kernel.cache_warmer | ||
| .............. |
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.
this should be something like:
Core Cache Warmers..................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.
Sorry about that, it's fixed. I kept the tag name for consistency with the rest of the tables below.
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.
the rest of the tables below refer to event names, not the tag names. Since cache warmers aren't event, they should not be treated the same
| The higher the priority, the sooner it gets executed. | ||
| Core Cache Warmers | ||
| .................. |
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.
This needs to be~~~~~~~~~ here. Small detail, but it's failing on Travis
Nice addition - thanks for the hard work Tony! |
This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes#3719).Discussion----------Fixed event listeners priority| Q | A| ------------- | ---| Doc fix? | yes| New docs? | no| Applies to | all| Fixed tickets | -Commits-------47d4d4f Fixed title level consistency1dbfd29 Updated title for the core cache warmersa9d07a5 Fixed title underline815f531 Replaced the priority paragraph with a note and a tabledb2c002 Fixed event listeners priority (typos after review)13b5645 Fixed event listeners priority