Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a sub-task of http://drupal.org/node/1775842 Convert all variables to state and/or config systems.
Comments
Comment #1
barbun CreditAttribution: barbun commentedAnd here's the small patch.
Comment #2
rbayliss CreditAttribution: rbayliss commentedLooks good, but shouldn't this be state? From the state() function's comment:
Comment #3
Albert Volkman CreditAttribution: Albert Volkman commentedI agree. Tagging.
Comment #4
Albert Volkman CreditAttribution: Albert Volkman commentedConverting to state system.
Comment #5
Lars Toomre CreditAttribution: Lars Toomre commentedI think we also should convert variable 'update_last_email_notification' to the state() system as well in this issue. Such a change would clean up all variable_get()'s in the Update module.
Comment #6
Albert Volkman CreditAttribution: Albert Volkman commentedGood call. I've added it to the meta issue, but I don't believe I can update the title of this issue.
Comment #7
alexpottThis should be migrating
update_last_email_notification
toupdate.last_email_notification
as well. In fact it'd be great if this could be bring the generalised method to update variables to state from #1798732: Convert install_task, install_time and install_current_batch to use the state system into this patch as that one is postponed.Comment #8
vijaycs85Please review
Comment #9
alexpottUpdating title
Comment #10
BerdirThe IN does not need to be specified explicitly, this is implicit if an array is passed in.
Also, this should be changed to use multiple lines:
Wrong indendation of $expected_state.
fields() should be on a separate line as well I think.
Also, there doesn't seem to be anything below the second comment?
Does this need to be Converts or not? also not sure about "state api value", maybe just "state API"? or "state system"? What do we use elsewhere?
Looks good to me otherwise.
Comment #11
BerdirCrosspost.
Comment #12
vijaycs85Thanks for the review Berdir, Please find the patch with review fixes.
Comment #13
alexpottRemoved bogus comment.
Comment #14
BerdirLooks good to me. Let's get this in, then the other state conversion issues can use the helper function and tests added here.
Comment #15
webchickGreat work! That update helper is gonna come in very handy.
Committed and pushed to 8.x.