-
Notifications
You must be signed in to change notification settings - Fork 48
agressively prefetch the menu as it needs to be completely loaded anyways #166
Conversation
dbu
commented
Dec 16, 2013
Q | A |
---|---|
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | travis |
Fixed tickets | partially #162 |
License | MIT |
Doc PR | symfony-cmf/symfony-cmf-docs#374 |
i guess we need to bump the dependency on the testing lib to 1.1? or use dev-master .. ? |
ok bumped it to 1.1 .. but now we have a new issue likely due to the recent CoreBundle merge:
|
ugh, i merged symfony-cmf/core-bundle#110 today, @dantleech added configuration options. so it seems the defaults are not set. @dantleech could you have a look and do a follow up PR? |
pr here: symfony-cmf/core-bundle#112 |
@lsmith77 okay, i triggered a rebuild on travis, lets see. |
|
@@ -32,7 +32,8 @@ public function getConfigTreeBuilder() | |||
->canBeEnabled() | |||
->children() | |||
->scalarNode('menu_basepath')->defaultValue('/cms/menu')->end() | |||
->scalarNode('content_basepath')->defaultValue('/cms/content')->end() | |||
->scalarNode('content_basepath')->defaultNull()->end() | |||
->scalarNode('prefetch')->defaultValue(10)->end() |
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.
TODO: update xml schema (wait for #167 to be merged)
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.
schema is updated.
alright, test is fixed hopefully. we do not test the actual prefetch activity however. |
@@ -33,6 +33,7 @@ public function getConfigTreeBuilder() | |||
->children() | |||
->scalarNode('menu_basepath')->defaultValue('/cms/menu')->end() | |||
->scalarNode('content_basepath')->defaultValue('/cms/content')->end() | |||
->scalarNode('prefetch')->defaultValue(10)->end() |
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.
scalarNode? Not integerNode ?
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.
oh, was not aware that integerNode even exists. thanks.
agressively prefetch the menu as it needs to be completely loaded anyways