variable_get($name, $default = NULL)

Moving out of the "x.y.z" queue to a real queue.

Yes please.

Changing to D7, but this could still get copied into the D6 branch. It's an API function, but the change is fully backward-compatible since all existing calls provide a default value.

The attached patch will make variable_get's second parameter have a default value of NULL. I searched for variable_get calls that passed NULL as the second parameter and removed the argument.

@arancaytar, this cannot go in D6, as modules that depend on this behavior will be minor version specific (only work with 6.2+). That kind of version dependency is not supported by the core attribute in .info, nor is a minor version dependency desired.

Patch applies cleanly to HEAD, and not being sure of what to test, I simply tried to do various. Nothing broke. I'm not confident enough to mark this RTBC though, but I'm fairly certain that's the status it is worthy of. :)

+1 null is used to mean "A value isn't available"
if it still applies then it is RTBC

freso@nayru /s/h/l/h/drupal7> patch -p0 < 88264.patch
patching file includes/bootstrap.inc
patching file includes/file.inc
patching file modules/color/color.module
patching file modules/locale/locale.install
patching file modules/profile/profile.module
patching file modules/system/system.install
Hunk #2 succeeded at 2425 (offset 110 lines).
patching file modules/user/user.module
freso@nayru /s/h/l/h/drupal7>

Still applies (though with some offset), so marking RTBC.

In what cases there would be no default?

Hi Dries, all the cases introduced by the patch are examples of when there doesn't need to be a default.

It's nice to have the default NULL because it saves you some typing when you don't care what the default is (NULL).

I've updated the patch to HEAD.

Only applies with fuzz now. I'd rather see #145164: DX: Use hook_variable_info to declare variables and defaults get in though.

Updated to latest HEAD, did a file search for all variable_get calls passing in NULL for the value, and removed the argument.

Catch, I would rather have #145164: DX: Use hook_variable_info to declare variables and defaults in as well, but I don't see it happening anytime soon. Would be good to at least get this small patch in.....

I'd still really like to see this patch go in, it's a sensible default.

Rerolled the patch against HEAD, caught a few more NULLs.

We could probably catch a few more, in places where the default is '' or 0, but NULL would be functionally equivalent.

I reviewed the code in the patch, and all looks well. It applies to the latest HEAD, and I tried several pages which call some variables, which were fine. In particular, I manually deleted the string in {system} cron_last, and the function still properly passes the if (!is_numeric($cron_last)) { $cron_last = variable_get('install_time', 0); }

I spotted several places where a function's param 2, i.e. "(, 0)" could be removed, but those don't hold up this particular patch from being committed. Go for it!

Sorry for setting it back to Code Needs Review, but I thought it wasn't awesome enough and needed a test. Hopefully this gets webchick's attention now....

It also seems like this is the appropriate first step towards #145164: DX: Use hook_variable_info to declare variables and defaults.

+1 for this simple feature.

As Rob Loach mentions, there's this other patch about declaring variables, which I know from my own experience (I put a lot of hours on it for Drupal 6) it needs to always be a much bigger patch because of this issue.

There are a lot of places in the code where defaults are not really needed and any patch attacking the issue of providing global defaults for variables will always bump into this issue, which is one of the reasons it always seems to get stuck.

A small step towards resolving one of our biggest DrupalWTFs (#145164: DX: Use hook_variable_info to declare variables and defaults)

Thanks for the tests. Committed to HEAD. Not sure about back-porting this to D6, because it means a (small, backwards-compatible) API change.

Please add to the upgrade docs.

Thanks Angie, you're my hero........... Update docs are up, if you're a better writer, then please feel free to hack at it.

Yay! Thanks, Rob Loach. :)