Part of #3566536: [meta] eliminate core .module files

Problem/Motivation

See #3566536: [meta] eliminate core .module files

Proposed resolution

Deprecate the automated_cron_settings_submit() function

Remaining tasks

None.

User interface changes

None.

Introduced terminology

None.

API changes

automated_cron_settings_submit() is deprecated

Data model changes

None.

Release notes snippet

Issue fork drupal-3566768

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: Unassigned » claudiu.cristea

claudiu.cristea opened merge request !14287

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Title: Deprecate automated_cron.module file » Deprecate automated_cron_settings_submit()
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: claudiu.cristea » Unassigned
Status: Active » Needs review
nicxvan’s picture

nicxvan commented
Status: Needs review » Needs work

Let's update this to use the new pattern in https://www.drupal.org/node/3548821

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review

Changed. Let's see the bot

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Can this just use #config_target?

https://www.drupal.org/node/3373502

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

Can this just use #config_target?

I have tried but it's tricky because the base form (form that we're altering) is not a Config form

nicxvan’s picture

nicxvan commented

I think since we have a shared cr we probably want the text in the remaining tasks worth a link since once that is published we need to wait for each issue to be merged to add the details.

I think since it doesn't extend config form base we don't want to use config target right?

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

I think since we have a shared cr we probably want the text in the remaining tasks worth a link since once that is published we need to wait for each issue to be merged to add the details.

Not sure I understand how to proceed

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

OK, applied the approach discussed on Slack

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

What else is needed here?

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

I think it's ready, I had just wanted to give @longwave a couple of days to respond to 10.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

alexpott made their first commit to this issue’s fork.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Deprecate automated_cron_settings_submit() » Change \Drupal\system\Form\CronForm to use ConfigFormBase and use #config_target
Status: Reviewed & tested by the community » Needs review

I think we should use #config_target here - to do this we need to make \Drupal\system\Form\CronForm extend from ConfigFormBase.

We get quite a bit of functionality with this change - all the #config_target stuff... but also the overridden config message too.

nicxvan’s picture

nicxvan commented

If you think that is with in scope I think we can do that!

I just rebased again to pull in the fix for the failure on head.

nicxvan’s picture

nicxvan commented

Wow that actually makes this a lot nicer looking!

Just waiting on a green run.

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

I think this is ready, I also pulled this down and double checked that the overridden message works and it does.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed a8fe3f24e96 to main and f2c0dd1902a to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !14287

  • longwave committed f2c0dd19 on 11.x 
    refactor: #3566768 Change \Drupal\system\Form\CronForm to use...

  • longwave committed a8fe3f24 on main 
    refactor: #3566768 Change \Drupal\system\Form\CronForm to use...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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