Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.- In session_cache_set, how about a 3rd parameter that deletes the bin? Then you wouldn't overload $data with doing a special task when it's NULL.
- I see you've got default cases in get/set, I assume that's to support add-ons to this module that want to define new methods, nice! Your readme goes into detail at the bottom, but what about adding a session_cache.api.php? https://drupal.org/coding-standards/docs#functions. You do have a nice example already.
- Instead of hook_cron to flush your caches, I think implementing a hook_flush_caches will let Drupal take care of expiring out of date item. Or maybe you need both?
- In session_cache_file.module you can use file_prepare_directory to simplify the logic a little.
Ok, those are all the nits I could pick :) What do you think?
Comments
Comment #1
rdeboerIn session_cache_set, how about a 3rd parameter that deletes the bin? Then you wouldn't overload $data with doing a special task when it's NULL.
To me that's a matter of choice. Personally I find $data=NULL a natural way to indicate that the bin may be deleted.
Also the API has been out for a while and I'm not keen on changing function signatures and surprising programmers across the world.
I see you've got default cases in get/set, I assume that's to support add-ons to this module that want to define new methods, nice! Your readme goes into detail at the bottom, but what about adding a session_cache.api.php? https://drupal.org/coding-standards/docs#functions. You do have a nice example already.
Have added session_cache.api.php
Instead of hook_cron to flush your caches, I think implementing a hook_flush_caches will let Drupal take care of expiring out of date item. Or maybe you need both?
No, you are right.
function system_cron()calls hook_flush_caches() to build a list and then flushes them all. Have removed hook_cron() implementation in favour of hook_flush_cashes() implementation. Thanks!In session_cache_file.module you can use file_prepare_directory to simplify the logic a little.
Yes that would be neater. I have added a @todo for when I'm next in that hood. Don't have the time to test all the implications of that change, right now.