Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
update.module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
2 Aug 2012 at 16:09 UTC
Updated:
29 Jul 2014 at 20:56 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
n3or commentedTitle.
Comment #2
n3or commentedThe main update module should be fully converted now. (Upgrade path, yml-file, replacement of variable_*(), naming of configuration options ..)
I removed the constants "UPDATE_MAX_FETCH_TIME" and "UPDATE_MAX_FETCH_ATTEMPTS" (update.module). They were used as variable_get default values only. I set their numeric values as default in update.settings.yml, so they are redundant.
This patch isn`t complete! I will add "update_test_*" and "update_script_*" transformation in next patch version.
Comment #3
n3or commentedForgot to add yml-file.
Comment #5
sunThe indentation for nested keys should be 4 spaces... :-/
Would it make sense to put all the fetch settings into a separate 'fetch' key on the top-level? E.g.:
Wow, I was horribly mistaken when I first saw this key and wanted to suggest to change it into 'enabled'...
Let's rename this into 'check.disabled_extensions'
I had to look up what this key means. Possible alternatives:
check.interval
check.interval_days
check.expire
check.expire_days
All not really ideal, better suggestions welcome.
This seems to be a simple array, so the empty syntax is
[]This variable is "state", not configuration, and thus we should leave it alone.
Let's use a if/else control structure instead of the ternary operator here, so we do not need to load the config object if $project contains a URL already.
&$form_state always needs to be taken by reference.
I do not understand why this was added? Can we revert it? :)
Comment #6
n3or commentedThank you!
What about something like "check.repeat_interval"?
I assumed this code from the original code. Can I remove it? I wasn't sure about that.
Comment #7
n3or commentedMade changes suggested by sun and converted update_test and update_test_script.
Comment #8
n3or commentedComment #9
sunUnfortunately, I overlooked this... this is also state, and/or something we need to resolve differently... let's just revert all the affected code back to variables... :-/
...
oh! This key doesn't really exist -- the code you've changed belongs to an example hook implementation in the API documentation for Update module. Let's remove the key from the config file, but keep the change to the example hook code :)
Comment #10
n3or commented#9 fixed.
Comment #11
n3or commented..
Comment #12
sunExcellent work. Almost done!
We need to re-introduce two variable_del()s for these two state variables. Ideally, along with a
Comment #13
n3or commentedComment #14
alexpottGreat patch! Couple of minor nitpicks...
This comment needs to be re-jigged so that each line uses as much of the 80 chars as possible and it should say "Checks the 'update_test.settings:system_info' configuration and..."
As above
Comment #15
n3or commented#14 done! :)
Comment #16
n3or commentedRefers to #14, forgot to replace "setting" by "configuration". Interdiff refers to patch #13.
Comment #17
aspilicious commentedtrailing whitespace
trailing whitespace
Almost every text editor/ide has options to trim trailing whitespaces on save. It makes your life easier ;)
26 days to next Drupal core point release.
Comment #18
n3or commentedArgh! Thank you, found and activated this option :)
Comment #19
n3or commentedComment #20
sun99.9% done! :)
Let's also remove the variable of the example hook implementation from the module update :)
Comment #21
n3or commentedComment #22
n3or commentedComment #23
sunAwesome!
Comment #24
dries commentedCommitted to 8.x, including the .yml file. Win! :-)
Thanks everyone.