- First commit to issue fork.
- @jonmcl opened merge request.
- last update
over 1 year ago Composer error. Unable to continue. - πΊπΈUnited States JonMcL Brooklyn, NY
I hope you all don't mind me creating merge request !5. It is for Drupal 10 compatibility and config_filter ^2.4 (necessary to Drupal 10 compatibility). !5 was made off of !4.
- π©πͺGermany tstoeckler Essen, Germany
Absolutely don't mind, no problem. Since the tests are still failing, I can't merge it as is. As far as I can tell, #15 is still accurate in terms of what could happen to push this forward.
- @tstoeckler opened merge request.
- π©πͺGermany tstoeckler Essen, Germany
Okay, took another stab at this and created a new MR. This time around I went ahead and converted the filter to a core storage transformer, so we don't need a dependency on Config Filter at all anymore. This would have been needed at some point anyway and in the case of this module it will actually be required to properly fix Config Split integration, see #3154531: Some issue when working together with config split β (although I have yet to comment there as of writing this).
I looked at the issue with installing from existing config again as described in #15 and I now found that this is in fact broken in core: Installing from existing config is not compatible with storage transformers. Attaching a tiny core patch that will fix that and make those tests pass. I will open a core issue for that, but will probably just mark those tests as skipped for now, as I don't think it makes sense to wait on a core bugfix.
With that I can actually make the tests green locally and the move to a storage transformer is also a really nice code simplification. It's good that I had split out most of the logic into the separate storage classes, as we can just use them in the same way now, so the amount of changes is not that big in fact.
Having two separate event subscribers would not strictly be necessary, but it will become necessary for the Config Split integration, so just went ahead and did that already. In fact it may be the case that the branch already works together with Config Split but I didn't test.
Note that I won't merge this as is yet, as there is a few things to figure out, as can maybe be seen from the code. Also this will be the starting point of a 2.x branch, and I will tag a stable 1.0 release before this.
But it's really not far away. Thanks everyone, in particular @bircher for getting this started!
- π©πͺGermany tstoeckler Essen, Germany
Oops, forgot the core path, here we go.
-
tstoeckler β
committed 03270041 on 2.x
Issue #3168037 by rwanth, tstoeckler, bircher, jurgenhaas, JonMcL:...
-
tstoeckler β
committed 03270041 on 2.x
- Status changed to Fixed
about 1 year ago 1:55am 21 October 2023 - π©πͺGermany tstoeckler Essen, Germany
Went ahead and merged now, thanks again everyone!
Automatically closed - issue fixed for 2 weeks with no activity.