Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andreiashu’s picture

Status: Active » Needs review
FileSize
710 bytes
andreiashu’s picture

Status: Needs review » Needs work
FileSize
710 bytes

tagging and back to needs work

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

andreiashu’s picture

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

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

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 :)