I think for this one we need a field.yml config file as well?

Files: 
CommentFileSizeAuthor
#8 1798782-9.patch2.6 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 42,826 pass(es). View
#5 1798782_field_purge_batch_size_3.patch734 bytesandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es). View
#2 1798782_field_purge_batch_size_1.patch710 bytesandreiashu
FAILED: [[SimpleTest]]: [MySQL] 41,671 pass(es), 1 fail(s), and 0 exception(s). View
#3 1798782_field_purge_batch_size_2.patch746 bytesandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,901 pass(es). View
#1 1798782_field_purge_batch_size_1.patch710 bytesandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es). View

Comments

andreiashu’s picture

Status: Active » Needs review
FileSize
710 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es). View
andreiashu’s picture

Status: Needs review » Needs work
FileSize
710 bytes
FAILED: [[SimpleTest]]: [MySQL] 41,671 pass(es), 1 fail(s), and 0 exception(s). View

tagging and back to needs work

edit: not sure how the attached patch for in this comment...

andreiashu’s picture

Issue tags: +Configuration system
FileSize
746 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,901 pass(es). View

YML file is now called field.settings.yml, updated code as well

yched’s picture

Status: Needs work » Needs review

We don't need 'field_' in the name of the new property - 'purge_batch_size' should be enough.
Other than that, this should be ready to fly.

andreiashu’s picture

FileSize
734 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,894 pass(es). View

thanks for the review Yves. New patch attached

yched’s picture

Status: Needs review » Reviewed & tested by the community

Thanks !

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

swentel’s picture

Status: Fixed » Needs review
FileSize
2.6 KB
PASSED: [[SimpleTest]]: [MySQL] 42,826 pass(es). View

Sorry te reopen, but the patch missed a couple of field_purge_batch functions which still are hardcoded to 10.

Stalski’s picture

Status: Needs review » Reviewed & tested by the community

Improvements are fine

longwave’s picture

Should the parameter to field_purge_batch be optional, and default to this config value, rather than having to repeat config()->get() everywhere?

swentel’s picture

Sounds plausible, there's no UI for this one anyway and probably never will be either. I can live with both situations, let's wait until a core committer checks the patch.

yched’s picture

Status: Reviewed & tested by the community » Fixed

Nope, the hardcoded 10s that remain are intentional. They didn't read from the variable before, ee still want them to be 10 even if someone puts the config value to 1000

swentel’s picture

Status: Fixed » Closed (fixed)

Oh, alright, sorry for the noise :)