CloudFlare Added Drupal Service Container #26
Conversation
aweingarten
force-pushed
the
cloudflare-service-aw1
branch
7 times, most recently
from
08c352b to
17bac23
Compare
| @@ -11,3 +11,8 @@ services: | |||
| arguments: ['%cloudflare.cache_tag_header_limit%'] | |||
| tags: | |||
| - { name: event_subscriber } | |||
| cloudflare: | |||
There was a problem hiding this comment.
i'd rename this cloudflare.service
One think you can think of is "what" the service is doing. Maybe its a manager or maybe its an engine or something. But, you may have a better term for this than service which is generic
There was a problem hiding this comment.
I buy into the namespacing in the name. Is .service any better than no namespace? If this is the paradigm we are buying into it probably needs to be a little more descriptive. cloudflare.basicsettings and cloudflare.invalidator
aweingarten
force-pushed
the
cloudflare-service-aw1
branch
from
17bac23 to
a6bedd0
Compare
| @@ -102,37 +102,7 @@ public function validatePathClear(array &$form, FormStateInterface $form_state) | |||
| * The current state of the form. | |||
| */ | |||
| public function purgeEntireZone(array &$form, FormStateInterface $form_state) { | |||
There was a problem hiding this comment.
This seems odd to me... a function with a one line call back?
There was a problem hiding this comment.
yes! This is actually legit
aweingarten
force-pushed
the
cloudflare-service-aw1
branch
18 times, most recently
from
7e5bc57 to
8b9c159
Compare
|
|
||
| catch (CloudFlareHttpException $e) { | ||
| drupal_set_message("Unable to clear zone cache." . $e->getMessage(), 'error'); | ||
| \Drupal::logger('cloudflare')->error($e->getMessage()); |
There was a problem hiding this comment.
Need to use the injected logger.
…ction. Purge functionality was deliberately not made into a service, as this should only be used via the purge module which will access that functionality via plugins.
aweingarten
force-pushed
the
cloudflare-service-aw1
branch
from
8b9c159 to
4b586f9
Compare
|
|
||
| // If no exceptions have been thrown then the request has been successful. | ||
| drupal_set_message("The zone: $zone was successfully cleared."); | ||
| $cloudflare_config = \Drupal::service('cloudflare.config'); |
There was a problem hiding this comment.
This service should be injected.
|
I think it may be helpful for us to get on an architecture call, perhaps with @nielsvm as well. |
|
closing in favor of #26 |
See issue: #16
Will probably bite the bullet and add automated tests here before merging.