follow-up to: #254491: Standardize static caching

see last patch examples: http://drupal.org/node/254491#comment-1430180 and also: http://drupal.org/node/224333#static_variable_api

Apply this conversion pattern to includes/menu.inc to convert all static variables there. Pay close attention to any place a reset parameter is provided and add a call to drupal_static_reset() where appropriate (e.g. in any calling function that uses the reset parameter)

CommentFileSizeAuthor
#2 jamesan_422368-2.patch4.06 KBJamesAn

Comments

pwolanin’s picture

Per discussion w/ catch - if you have a variable name more complex than just __FUNCTION__, use a ':' to separate the suffix (this will avoid colliding with any other valid function name). e.g. :

&drupal_static(__FUNCTION__ . ':second_var');
JamesAn’s picture

StatusFileSize
new4.06 KB

I noticed menu_get_names() is never called or mentioned in the code anywhere. Is that function unused?

I also modified the condition of an if-statement in _menu_clear_page_cache() from empty($cache_cleared) to $cache_cleared == 0 as I think the equality comparison better indicates that $cache_cleared is a counting variable.

JamesAn’s picture

Status: Active » Needs review

Always forgetting to flip the status.. -_-

pwolanin’s picture

menu_get_names() is a API function provided by core, but indeed not used in core. It's actually quite different in function (though perhaps too similar in name) to menu_get_menus(). I don't think it should be removed, but perhaps named better. Likely there are other parts of the menu system where we shoudl provide more such APi functions to reduce the necessity for contrib modules going directly to the DB.

pwolanin’s picture

Status: Needs review » Reviewed & tested by the community

patch still applies cleanly, works fine, all straight-forward conversions.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.