Referring to
#1775842: [meta] Convert all variables to state and/or config systems

Converting node_cron_comments_scale.

Files: 
CommentFileSizeAuthor
#18 comment-scale-state-1831486-18.patch3.41 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 48,038 pass(es).
[ View ]
#15 1831486_convert_comment_variables_with_tests_and_uninstall_15.patch3.37 KBmiro_dietiker
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1831486_convert_comment_variables_with_tests_and_uninstall_15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 1831486_convert_comment_variables_with_tests_and_uninstall_12.patch3.05 KBmiro_dietiker
PASSED: [[SimpleTest]]: [MySQL] 47,839 pass(es).
[ View ]
#11 1831486_convert_comment_variables_with_tests_and_uninstall_11.patch3.06 KBmiro_dietiker
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/tests/upgrade/drupal-7.state.system.database.php.
[ View ]
#6 1831486_convert_comment_variables_with_tests_and_uninstall.patch3.38 KBmiro_dietiker
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1831486_convert_comment_variables_with_tests_and_uninstall.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 1831486_convert_comment_variables_with_tests.patch3.06 KBmiro_dietiker
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es).
[ View ]
#1 1831486_convert_comment_variables.patch1.66 KBmiro_dietiker
PASSED: [[SimpleTest]]: [MySQL] 47,561 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.66 KB
PASSED: [[SimpleTest]]: [MySQL] 47,561 pass(es).
[ View ]

First try!

StatusFileSize
new3.06 KB
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es).
[ View ]

Now with test coverage.

Issue tags:+Configuration system

Tagging...

Status:Needs review» Needs work

hmm in the state api you can't just replace 'node_cron_comments_scale' with 'comment.count_scale'. Because nobody will know it has something to do with nodes on cron. You should just replace variable_set/variable_get with state()->set and state()->get.

@aspilicious: We can't just replace variable_get() with state, because are required to prefix it with the module. So it would be 'comment.node_cron_comments_scale'. Which is very long. And IMHO wrong, because it doesn't have to do with nodes on cron. It is based on the highest count of comments a single node has.

So maybe comment.count_scale is too short, but I'm not sure what would be better.

StatusFileSize
new3.38 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1831486_convert_comment_variables_with_tests_and_uninstall.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Providing new version. Now with uninstall.

SELECT MAX(comment_count) FROM {node_comment_statistics}

Renamed to comment.node_comment_statistics_scale since it's a query against {node_comment_statistics}. And yes, nothing to do with cron.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

This looks good to me. The state name is now consistent with the meaning IMHO, makes sense to use the same as the table that we're calculating the scale for.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Configuration system

The last submitted patch, 1831486_convert_comment_variables_with_tests_and_uninstall.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.06 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/tests/upgrade/drupal-7.state.system.database.php.
[ View ]

Rerolled.

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new3.05 KB
PASSED: [[SimpleTest]]: [MySQL] 47,839 pass(es).
[ View ]

Bad merge :-) retry!

Status:Needs review» Needs work

State is not automatically cleaned up on uninstall (at the moment) so we should add a state('')->delete() to comment_uninstall(). And yep this was missing for the current variable.

Otherwise looks good.

Status:Needs work» Needs review
StatusFileSize
new3.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1831486_convert_comment_variables_with_tests_and_uninstall_15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Hmm... #6 had uninstall // delete already. So reroll #11 / #13 where wrong.

Now #6 rerolled cleanly.

Status:Needs review» Needs work
Issue tags:+Configuration system

Status:Needs work» Needs review
StatusFileSize
new3.41 KB
PASSED: [[SimpleTest]]: [MySQL] 48,038 pass(es).
[ View ]

Re-rolled.

Status:Needs review» Reviewed & tested by the community

Looks good now

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

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