New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a ConfigUpdater service #347
Conversation
2a03c19
to
231656e
Compare
@bojanz ready for review :) and proper commit name suggestion |
231656e
to
3e3ffe2
Compare
@@ -0,0 +1,53 @@ | |||
<?php | |||
/** | |||
* Created by PhpStorm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docblock needs to go :)
@@ -34,6 +34,10 @@ services: | |||
tags: | |||
- { name: service_collector, tag: commerce.availability_checker, call: addChecker } | |||
|
|||
commerce.config_update: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the name of the class is ConfigUpdater, then the service name should match, commerce.config_updater
public function delete(array $config_names); | ||
|
||
/** | ||
* Gets the current active value of configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Gets/Loads, so that the verb matches the method name.
Looks almost ready. Great job! |
74fb1c8
to
0304edc
Compare
@bojanz all items addressed |
e18c9cd
to
4af7656
Compare
Because hook_update_N + config == hard.
4af7656
to
7f9cf74
Compare
$type = $this->getConfigType($config_name); | ||
if ($type == 'system.simple') { | ||
$config = $this->configFactory->getEditable($config_name); | ||
if (!$config) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getEditable() always returns a config object, so the if (!$config) will never work. Furthermore, we already checked active storage above.
Addressed most of the feedback above, committed in af4e4bb. Followup:
|
Because hook_update_N + config == hard.