Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The code in actions.inc is in the process of being converted to a new actions module in #1008166: Actions should be a module. When that is complete, the variable 'actions_max_stack' (and possibly others) in that new module will need to be converted to use the new CMI sub-system.
Comment | File | Size | Author |
---|---|---|---|
#25 | drupal-1788084-25.patch | 1.79 KB | dawehner |
#17 | 1788084-convert_actions_variables_to_CMI_add_upgrade.patch | 512 bytes | andyceo |
#12 | drupal-1788084-12.patch | 257 bytes | dawehner |
#8 | 1788084-convert_actions_variables_to_CMI-8.patch | 2.64 KB | andyceo |
#5 | 1788084-convert_actions_variables_to_CMI-2.patch | 2.15 KB | andyceo |
Comments
Comment #1
Lars Toomre CreditAttribution: Lars Toomre commentedIn the issue summary, the possibility of other action information about being converted to CMI sub-system was raised. I am wondering whether configuration information for configurable actions should be converted directly to config files or if that info should be part of a config object associated with a action configurable plugin. Thoughts?
Comment #2
sunComment #3
andyceo CreditAttribution: andyceo commentedMy first work with new CMI system.
Comment #4
sunYay, thanks! :)
1) The config file/object should be named 'action.settings'.
2) 'max_stack' doesn't sound very self-descriptive to me. It's about stack overflow, and infinite recursion, ... I'm fairly sure that there is a better and more common name for such a parameter in the tech industry.
Comment #5
andyceo CreditAttribution: andyceo commentedfixed.
As far as I know, the similar thing is the recursion limit in python language:
The function in python can not call itself more than 1000 times.
So I replaced "max_stack" with "stack_limit".
Comment #6
sunWhy don't we simply name it
recursion_limit
then? :) That sounds most self-descriptive, at least to me.Comment #7
andypost+1 to recursion_limit - self descriptive
Use a variable here like
Also I suggest to change a watchdog() message to contain a variable to describe a limit.
A kind of
"Stack overflow: recursive calls to actions_do() is limited by %limit... ", array('%limit' => $recursion_limit
Use chain call without variable:
Comment #8
andyceo CreditAttribution: andyceo commentedThank you all for your help!
Here is reworked patch.
Comment #9
sunThanks! :)
Comment #10
webchickThanks a lot!
Committed and pushed to 8.x.
Comment #12
dawehnerThe yml file seems to be missing from the directory.
Comment #13
sunComment #14
damiankloip CreditAttribution: damiankloip commentedYep, we need this in. I have added it to the patch in #1828410: Provide a bulk_form element for actions and fixes the issue.
Comment #15
webchickCommitted and pushed to 8.x. Thanks!
Comment #16
andyceo CreditAttribution: andyceo commentedDid we forget the upgrade path here?
Comment #17
andyceo CreditAttribution: andyceo commentedHere is patch with update path.
Comment #19
dawehnerI guess we should also add an upgrade path test for that.
Comment #20
andypost#17: 1788084-convert_actions_variables_to_CMI_add_upgrade.patch queued for re-testing.
Comment #21
andypost@dawehner I see no reason to add upgrade test - this issue about 1 variable not a table
Comment #23
catchComment #24
dawehnerWorking on that
Comment #25
dawehnerIn contrast to the patch in #17 i moved this into system.install because previously the functionality of actions.module happened in includes/actions.inc
Comment #26
aspilicious CreditAttribution: aspilicious commentedComment #27
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.