Updated: Comment #N

Problem/Motivation

When SystemConfigFormBase was added in #1921996: Convert system_config_form() to implement FormInterface as a base class., we named it directly after system_config_form() and left it in system.module, because there was only \Drupal\Core\Form\FormInterface in that directory.

Now it can live alongside ConfirmFormBase and FormBase

Proposed resolution

Move \Drupal\system\SystemConfigFormBase to \Drupal\Core\Form\ConfigFormBase

Remaining tasks

N/A

User interface changes

N/A

API changes

If you extend SystemConfigFormBase, you'll have to fix your use statements.

N/A

CommentFileSizeAuthor
#1 config-form-base-2089627-1.patch22.19 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
22.19 KB
webchick’s picture

Issue tags: +pants

Tagging.

jibran’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -pants

Now it can live alongside ConfirmFormBase and FormBase

happily ever after. :D

webchick’s picture

Assigned: Unassigned » webchick
Status: Reviewed & tested by the community » Fixed

<3 <3 <3

Committed and pushed to 8.x. Thanks!

I'll take care of the change notice.

webchick’s picture

Assigned: webchick » Unassigned
Issue tags: +pants

Hooray, done. :)

jibran’s picture

Assigned: Unassigned » webchick

Tagging.

jibran’s picture

Assigned: webchick » Unassigned

x-post.

mondrake’s picture

Seems not pushed yet?

mondrake’s picture

Priority: Normal » Critical
Status: Fixed » Needs work

@webchick sorry I do not know how else to raise this, seems like this is not pushed to HEAD... or I am missing sth

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

Still applies.

webchick’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Oops! So sorry. :(

Really committed and pushed to 8.x this time. :) Thanks!

tstoeckler’s picture

Although this is a form, I don't see how this fundamentally belongs to the forms system. I think \Drupal\Core\Config\Form would be a much more natural fit. Thoughts?

tim.plunkett’s picture

That's not a namespace that exists. And we're not going to have \Drupal\Core\Confirm\Form\ConfirmFormBase or anything...

tstoeckler’s picture

Well, OK I was a bit terse with my comment. The "Form" namespace below \Drupal\Core\Config does not exist. But the "Config" system is actually a thing, and to me it seems a thing like "ConfigFormBase" belongs to it. That was my suggestion.

There's no such thing as a "Confirm" system. That's just silly.

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