This is part of #1924990: [meta] Convert all of system_config_form() to SystemConfigFormBase.

CommentFileSizeAuthor
#10 drupal-aggregator_config_form-1925048-10.patch13.23 KBParisLiakos
#6 aggregator-1925048-6.patch16.2 KBamateescu
#4 aggregator-1925048-4.patch17.54 KBtim.plunkett
#3 aggregator-1925048-3.patch25.88 KBtim.plunkett
#1 aggregator-1925048-1.patch25.88 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » tim.plunkett
Status: Active » Needs review
FileSize
aggregator-1925048-1.patch25.88 KB

This is an example of a more advanced conversion, since the form has dependencies other than config.factory.

It contains the patch from #1921996: Convert system_config_form() to implement FormInterface as a base class. for now.

Status: Needs review » Needs work

The last submitted patch, aggregator-1925048-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
aggregator-1925048-3.patch25.88 KB

Trailing semicolon

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
aggregator-1925048-4.patch17.54 KB

Those other issues were committed.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: tim.plunkett » Unassigned
Status: Needs review » Needs work
Issue tags: +SprintWeekend2013

This needs a reroll, use #1925738: Convert language's system_config_form() to SystemConfigFormBase as a guide

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs work » Needs review
FileSize
aggregator-1925048-6.patch16.2 KB

Rerolled.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

hmmm this conflicts a bit with #1930274: Convert aggregator processors and parsers to plugins where, btw i removed sanitize_configuration..dunno which should get in first

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs review » Postponed

Probably that one, this form conversion is pretty easy to reroll/redo.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Assigned: Unassigned » ParisLiakos
Status: Postponed » Needs work
ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs work » Needs review
FileSize
drupal-aggregator_config_form-1925048-10.patch13.23 KB

thats another nice cleanup, thanks @tim.plunkett for SystemConfigFormBase awesomeness:)

Status: Needs review » Needs work
Issue tags: -FormInterface, -SprintWeekend2013

The last submitted patch, drupal-aggregator_config_form-1925048-10.patch, failed testing.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs work » Needs review
Issue tags: +FormInterface, +SprintWeekend2013

Drupal\translation_entity\Tests\EntityTranslationUITest->testTranslationUI() line 40
/me smells random failure
#10: drupal-aggregator_config_form-1925048-10.patch queued for re-testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Reviewed & tested by the community

This one ready!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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