F/drupal11 - #346
Conversation
jekuaitk
left a comment
There was a problem hiding this comment.
A couple questions otherwise good!
| "drupal/advancedqueue": "^1.0", | ||
| "drupal/cache_control_override": "^1.1 || ^2.0", | ||
| "drupal/coc_forms_auto_export": "^3.0@alpha", | ||
| "drupal/config_entity_revisions": "2.0.x-dev", |
There was a problem hiding this comment.
There was a problem hiding this comment.
@ds-bellcom as per our agreement, config_entity_revisions is going to be removed. But as Jeppe has correctly mentioned, there is a code dependant on it. This code is actually dedicated for facilitating the installations with config_entity_revisions enabled.
As i can see this dependancy can be quickly removed, but i would like to have and agreement on it :)
There was a problem hiding this comment.
There was a problem hiding this comment.
I had another look at at that. Here are some things i would like to clarify now.
@jekuaitk Do I understand correctly that ITK might still be using this module/modules in their solution?
If yes, then I suggest we leave this code as is, because it's already checking if webform_revisions is enabed, and only intercepts the method if module is enabled.
If that module is not enabled/existing it would call a standard getColumns implementation, which knows nothing about the revisions:
https://github.com/OS2Forms/os2forms/blob/develop/modules/os2forms_encrypt/src/WebformOs2FormsEncryptSubmissionStorage.php#L39-L42
If however ITK is not using these modules and not planing to, we can safely remove the override of this method getColumns.
Please tell us you're thoughts on that and i will make the proper changes accordingy.
cc @ds-bellcom
There was a problem hiding this comment.
@stankut We do indeed use all of them. Leaving the code as is and checking if the module is enabled seems reasonable to me!
Drupal 11 compatibility