Fixed event listeners priority #3719
Conversation
tony-co
commented
Mar 25, 2014
tony-co
commented
| 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.
"vary" because it's listener priorities, no?
There was a problem hiding this comment.
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.
this should be something like:
Core Cache Warmers
..................There was a problem hiding this comment.
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.
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.
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 consistency 1dbfd29 Updated title for the core cache warmers a9d07a5 Fixed title underline 815f531 Replaced the priority paragraph with a note and a table db2c002 Fixed event listeners priority (typos after review) 13b5645 Fixed event listeners priority
