Subissue of #1787218: Convert testing variables to state system..
Change the variable xmlrpc_test_xmlrpc_alter to the state system.

CommentFileSizeAuthor
#9 1848066-xmlprc_to_state-drupal8-9.patch1.9 KBACF
#7 1848066-xmlprc_to_state-drupal8-7.patch18.91 KBACF
#4 1848066-xmlprc_to_state-drupal8-4.patch1.89 KBGaelan
#1 1848066-xmlprc_to_state-drupal8-1.patch1.88 KBACF

Comments

ACF’s picture

ACF commented
Status: Active » Needs review
StatusFileSize
new 1848066-xmlprc_to_state-drupal8-1.patch1.88 KB

Patch attached.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Needs review » Needs work 
+++ b/core/modules/xmlrpc/tests/modules/xmlrpc_test/xmlrpc_test.moduleundefined
@@ -68,7 +68,7 @@ function xmlrpc_test_xmlrpc() {
-  if (variable_get('xmlrpc_test_xmlrpc_alter', FALSE)) {
+  if (state()->get('xmlrpc_test.xmlrpc_alter')) {

This needs a default value.

Otherwise looks fine.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented

I'm sorry I'm stupid. The state() system does not use default values like the variable system does. This needs to be converted to something like

$xmlrpc_alter = state()->get('xmlrpc_test.xmlrpc_alter') ?: FALSE;
if ($xmlrpc_alter) {

I'd rather not put the ternary in the if but that's just me.

I also wouldn't mind seeing the name changed to 'xmlrpc_test.alter' because the second xmlrpc seems redundant.

Gaelan’s picture

Gaelan commented
StatusFileSize
new 1848066-xmlprc_to_state-drupal8-4.patch1.89 KB

Fixes concerns in #3.

Gaelan’s picture

Gaelan commented
Status: Needs work » Needs review

I am forgetful.

Status: Needs review » Needs work

The last submitted patch, 1848066-xmlprc_to_state-drupal8-4.patch, failed testing.

ACF’s picture

ACF commented
Status: Needs work » Needs review
StatusFileSize
new 1848066-xmlprc_to_state-drupal8-7.patch18.91 KB

Attached a patch with the changes requested.

I'm not sure why the state()->get needs a default value, as it was in the if statement and it will fail the conditional statement either with false or null?

Status: Needs review » Needs work

The last submitted patch, 1848066-xmlprc_to_state-drupal8-7.patch, failed testing.

ACF’s picture

ACF commented
Status: Needs work » Needs review
StatusFileSize
new 1848066-xmlprc_to_state-drupal8-9.patch1.9 KB

Stupid mistake on the last patch.

aspilicious’s picture

aspilicious commented
Status: Needs review » Reviewed & tested by the community

Ok good to go!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Category: feature » task
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.