Added missing mandatory parameter #6122
Conversation
yceruto
commented
Jan 9, 2016
yceruto
commented
| Q | A |
|---|---|
| Doc fix? | yes |
| New docs? | no |
| Applies to | all |
| Fixed tickets | n/a |
| @@ -203,7 +203,7 @@ must also define additional blocks: | |||
| </div> | |||
| {% endset %} | |||
|
|
|||
| {{ include('@WebProfiler/Profiler/toolbar_item.html.twig') }} | |||
| {{ include('@WebProfiler/Profiler/toolbar_item.html.twig', {'link': profiler_url}) }} | |||
There was a problem hiding this comment.
I think we should use either { link: true } or { link: false } here. Using profiler_url here looks like you could pass the actual URL here.
There was a problem hiding this comment.
I agree with @xabbuh here.
There was a problem hiding this comment.
@xabbuh I agree.
I think this variable could be named show_link instead?
There was a problem hiding this comment.
@xabbuh however, the symfony core uses profile_url instead true:
|
👍 |
| @@ -203,7 +203,7 @@ must also define additional blocks: | |||
| </div> | |||
| {% endset %} | |||
|
|
|||
| {{ include('@WebProfiler/Profiler/toolbar_item.html.twig') }} | |||
| {{ include('@WebProfiler/Profiler/toolbar_item.html.twig', {'link': true}) }} | |||
There was a problem hiding this comment.
Or shouldn't we better set this to falselike it is done in the example before? Otherwise it might look strange for the reader that we changed the value.
There was a problem hiding this comment.
It should be true because the example requires access to profile panel. In the example before, the link is not necessary.
|
perhaps we need to add any comments to talk about the |
|
👍 |
This PR was submitted for the 2.7 branch but it was merged into the 2.3 branch instead (closes #6122). Discussion ---------- Added missing mandatory parameter | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | n/a Commits ------- 0368933 Added parameter mandatory
* remove wrong default value description * fix code style[#6122] some tweaks
|
Thanks @yceruto, I have merged your PR into the |
|
👍 |
