Problem/Motivation

Avoid any core or page update to be blocked by a patch that does not apply, mantainers will release new version as asap when that occurs.

Proposed resolution

Add suggest to composer.json & include documentation instead providing patches directly?
Add to status report page message? TBD

Issue fork vlsuite-3362621

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

CRZDEV created an issue. See original summary.

crzdev’s picture

crzdev commented
Assigned: Unassigned » alejandro cabarcos

Alejandro Cabarcos made their first commit to this issue’s fork.

alejandro cabarcos’s picture

alejandro cabarcos commented

Im not sure how we should handle the patches dependency. After reading composer docs and checking other contrib modules, the suggest composer property is used to suggest packages that can improve/boost/help this package.

https://getcomposer.org/doc/04-schema.md#suggest

For example, we could use this property to recommend the Ajax Loader module, but in this case, we need these patches to be applied.

I think we should provide help so those issues are fixed, and until then, create forks for these modules with the patches applied.

What do you think?

crzdev’s picture

crzdev commented

Maybe we should reconsider this with more time, those patches are essential in order to get suite working as expected (removing them may even make some future unit test impossible to pass). Changing MUST to SHOULD to launch first stable version.

The unique point of having them included is that may block some site update due patch does not apply in composer until is adjusted in suite (& we should get notified due automated testing already set for D9 & D10 stable versions in case that happens, even we can add other with dev versions in order to have more time to make any adjustment).

tunic’s picture

tunic
he/him
Primary language Spanish
Location Madrid
commented

I think there 3 options:

  1. Add patches to VLSuite's composer.json: installation is automatic and everything works but it will fail when patches are committed upstream
  2. Add patches to the README and/or VLSuite module page: installation is not completely automatic but it won't fail because patches are committed upstream.
  3. Add patches to VLSuite's composer.json but require specific versions on dependencies, versions we know patches apply: installation is automatic and no error when patches are committed upstream, but we will force VLSuite users to use certain module versions.

Option 3 is nto acceptable if we have to patche the core, we can't force users to use a certain core. WE would have to release a new VLSuite release on each core release.

Option 1 seems handy but it will break in the future.

So I guess the only option is 2. We may use the composer's Drupal Project Message Plugin to display a message about the required patches. Also, we have to list the patches oin the README and in the module's page, ideally with a little explanation on why the patch is needed and for what functionality.

tunic’s picture

tunic
he/him
Primary language Spanish
Location Madrid
commented

One of the biug drawbacks of option 2 is testing coverage. I thought we could not test anything that require a patch. However, I've just discovered that you can run commands before tests, so we may go with option 2 having complete test coverage.

https://www.drupal.org/drupalorg/docs/drupal-ci/customizing-drupalci-tes...

crzdev’s picture

crzdev commented
Version: 1.0.x-dev » 1.1.x-dev
Issue summary: View changes
mortona2k’s picture

mortona2k commented

The patch in the module conflicts with drupal 10.3.

crzdev’s picture

crzdev commented

Thanks @mortona2k, identified patches that does not apply into 10.3.0-beta1:

Working on that asap!

crzdev’s picture

crzdev commented
Related issues: +#3045509: EntityFieldManager key/value field map gets out of sync, doesn't recognise bundle fields
crzdev’s picture

crzdev commented
Related issues: +#3417795: Reorder Layout Builder sections

3034979 will be replaced by 3045509, created MR & applied into dev version to do some tests.
Other 3080606 is for 3417795 (postponed & not mandatory for stock vlsuite installation).

crzdev’s picture

crzdev commented

Re-roll for 10.3.0 of 3080606 (reorder sections core patch) up into that issue

mortona2k’s picture

mortona2k commented

Thanks I was able to install on 10.3 without composer errors.

crzdev’s picture

crzdev commented

Release 1.1.7 ready for core 10.3.0 compatibility!

mortona2k’s picture

mortona2k commented

This patch has been merged into Section Library and is no longer needed.

"drupal/section_library": {
    "3217647: Adding stuff to library doesn't close dialog if dialog opens in popup": "https://www.drupal.org/files/issues/2021-06-07/section_library.3217647.close-dialog.2.patch"
}

https://www.drupal.org/project/section_library/issues/3217647

mortona2k’s picture

mortona2k commented
Version: 1.1.x-dev » 2.x-dev

mortona2k opened merge request !57

crzdev’s picture

crzdev commented

Great, thanks @mortona2k, minimal required version constraint should be "^1.2", releasing asap!

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

  • crzdev committed 5b5ca68f on 2.x authored by mortona2k 
    Issue #3362621 by mortona2k, alejandro cabarcos, omarlopesino, crzdev:...
crzdev’s picture

crzdev commented

Integrated changes, another patch down!

crzdev’s picture

crzdev commented

2.2.1 Out!