🇺🇸United States @trackleft2

Tucson, AZ 🇺🇸
Account created on 2 April 2014, about 11 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 created an issue.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Actually, I might be wrong about this, because I saw this on a different issue.:

With this removal of any Composer constraint on Drupal core, this will mean that Composer will happily allow installing this version regardless of the Composer requirement for Drupal core. (If this module didn't have a composer.json file at all, then drupal.org packaging would use the info.yml file to generate Composer constraints, but since there is a composer.json file, this doesn't happen).

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

IMHO, the real solution would be to update the menu_block.post_update.php file with a new function to add the new key for sites that installed the module prior to the new key existing.

See https://git.drupalcode.org/project/menu_block/-/blob/8.x-1.x/menu_block.... for an extremely similar example.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Possible scenarios where cache should be invalidated:
- When the config_sync update mode has changed.
- When new managed configuration files appear or are removed from the codebase.
- When active configuration changes.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3439698-add-configdistro-status to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

See the Merge Request on 🐛 Not compatible with search_current_language module Active with a PHPUnit test that depends on another drupal module via the composer.json.

Additionally, we don't have to set the version constraint in composer.json, we can leave that off if necessary.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

If this is merged it would be good to add the new feature to the release notes and a scenario or two in which to use the setting.
How do we expect this to work, when the paths setting is negated?

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I haven't been able to figure this out.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3431903-automated-drupal-11-trackleft2 to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Oops, I saw the composer.json when I downloaded the repo.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

While working on other issues I found that this module does not contain a composer.json which makes it difficult to work in the way that I would like to locally. For example here is my .lando.yml file:

name: media_migration
recipe: drupal10
config:
  php: '8.3'
  via: apache:2.4
  webroot: web
  database: mariadb:10.6
  xdebug: false
services:
  database:
    type: compose
    services:
      image: mariadb:10.6
      command: docker-entrypoint.sh mariadbd
      restart: always
      ports:
        - '3306'
      environment:
        MARIADB_ALLOW_EMPTY_ROOT_PASSWORD: 'true'
        MARIADB_DATABASE: drupal10
        MYSQL_DATABASE: drupal10
        MARIADB_USER: drupal10
        MARIADB_PASSWORD: drupal10
  appserver:
    overrides:
      environment:
        SIMPLETEST_DB: 'mysql://drupal10:drupal10@database/drupal10'
        SIMPLETEST_BASE_URL: 'http://appserver'
      volumes:
        # Don't share our host working directory as /app. We want /app empty for composer.
        - /app
        # Instead share our host working directory as a standalone package.
        - .:/usr/local/media_migration
    build:
      # Create a new Drupal project and use the module as a non-packagist repository.
      - composer create-project --dev drupal/recommended-project:11.1.x /app
      - composer config extra.enable-patching true
      - composer config extra.composer-exit-on-patch-failure true
      - composer config allow-plugins.cweagans/composer-patches true
      - composer require cweagans/composer-patches
      - composer config minimum-stability dev
      - composer config allow-plugins.phpstan/extension-installer true
      - composer require --dev drupal/core-dev:^11.1 drush/drush phpspec/prophecy-phpunit:* phpstan/extension-installer mglaman/phpstan-drupal phpstan/phpstan-deprecation-rules drupal/config_inspector drupal/devel drupal/gin
      - composer config repositories.localdev path /usr/local/media_migration && composer require drupal/media_migration:\*@dev
  node:
    type: node:20
    build:
      - yarn install
  chromedriver:
    type: compose
    services:
      image: seleniarm/standalone-chromium:4.1.4-20220429
      command: /opt/bin/entry_point.sh
tooling:
  # Provide a command to install Drupal.
  install:
    service: appserver
    cmd:
      - /app/vendor/bin/drush --root=/app/web site:install --account-mail=noreply@example.com --account-name=admin --account-pass=admin --db-url=mysql://drupal10:drupal10@database:3306/drupal10 -y --verbose
      - /app/vendor/bin/drush en -y media_migration devel config_inspector
  # Provide Drush tooling to automatically know the Drupal root.
  drush:
    service: appserver
    cmd: /app/vendor/bin/drush --root=/app/web
  phpcs:
    service: appserver
    cmd: /app/vendor/bin/phpcs -s --colors --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/contrib/media_migration
  # Provide PHPCBF tooling to fix coding standards.
  phpcbf:
    service: appserver
    cmd: /app/vendor/bin/phpcbf -s --colors --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/contrib/media_migration
  # Provide phpstan tooling to check for code quality and deprecated code.
  phpstan:
    service: appserver
    cmd: /app/vendor/bin/phpstan analyse --configuration web/modules/contrib/media_migration/phpstan.neon web/modules/contrib/media_migration
  # Provide phpunit tooling to run unit tests.
  phpunit:
    service: appserver
    cmd: /app/vendor/bin/phpunit --configuration /app/web/core/phpunit.xml.dist --bootstrap /app/web/core/tests/bootstrap.php /app/web/modules/contrib/media_migration
  yarn:
    service: node
    cmd: yarn
  eslint:
    service: node
    cmd: yarn run eslint --color

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

One thing that I am not clear on is:
If a recipe alters a single line within config
Does that make the entire configuration overridden or can I merge upstream changes around the recipe provided config.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I wonder if we should address the recipe question on the project page.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸
🇺🇸United States trackleft2 Tucson, AZ 🇺🇸
🇺🇸United States trackleft2 Tucson, AZ 🇺🇸
🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 created an issue.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸
🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Let's merge this, we were planning on deprecating config_distro_filter, but I think at this point a D11 release is more useful.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Can we please have a new release of this module with Drupal 11 compatibility?

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I can confirm this is an issue, and found that the minimum Drupal core version that contains packageManager by default is 10.2.5

See https://git.drupalcode.org/project/drupal/-/blob/10.2.4/core/package.jso...
vs
https://git.drupalcode.org/project/drupal/-/blob/10.2.5/core/package.jso...

I think we can remove all of the help text that references packageManager

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I think this needs a post update hook to set the default value of the new config key.
Also, would be great to add a test.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Converted to merge request/

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

@musa.thomas Also, you need to use drupal-lenient if would like to install on Drupal 11 via composer as far as I can tell https://www.drupal.org/docs/develop/using-composer/using-the-lenient-com...

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

The views display sequence should be ordered by position, which obviously is not currently possible https://git.drupalcode.org/issue/drupal-2855675/-/blob/11.x/core/profile...

Here is the structure of the configuration:

display:
  default:
    position: 0
  page_1:
    position: 1

Here is the structure of the schema:

views.view.*:
  type: config_entity
  label: 'View'
  mapping:
    display:
      type: sequence
      label: 'Displays'
      sequence:
        type: mapping
        label: 'Display settings'
        mapping:
          position:
            type: integer
            label: 'Position'
🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Setting as needs review to discuss what to do about this new PHPUnit test.

I'd also like some feedback on the overall approach.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

After investigating the failing PHPUnit tests, it appears the issue is caused by unpredictable sequence order when configuration is saved without an orderby: key setting in the configuration's schema. As noted in the configuration schema documentation , sequence data is preserved in the order it exists unless explicitly sorted.

My test uses PHP’s shuffle function to alter the sequence order artificially. Without an orderby: key setting in the schema for view displays, the order remains unpredictable, which is causing the PHPUnit tests to fail.

To test this, I added orderby: key or orderby: value to a local codebase in the following locations:

Additionally, please note that there is an open dependency array order issue: #3186905: Normalize config import .

The Test will always fail unless we change the fixtures to remove sequences that aren't sorted, skip shuffling unsorted sequences, or some other way.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

That nitpick should be resolved in 📌 Use dependency injection instead \Drupal calls. Needs review

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

Sorry about the noise.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3230398-remove-plugin-system to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3513597-add-composer.json-file to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3.0.x to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3.0.x to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I think that makes sense.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

The two composer jobs are failing due to the lack of a Drupal 11 compatible version of features.
Personally, I think having the ability to test interoperability between this module and features is valuable, however if features doesn't create a compatible release, we'll be stuck with broken jobs.

We could consider just copying the parts of features we need to test into our test module.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

I've tested the changes in Merge Request !14 and they do appear to resolve the issue.

How I tested.(with the patch applied)

- I changed the update mode to "Full Reset"
- I ran drush cd-update --update-mode=1
- I saw that the update mode was overridden via the drush command to merge mode.

I removed the patch

- I ran drush cd-update --update-mode=1

I saw The "--update-mode" option does not exist.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

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

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3144104-new-api-for to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 8.x-2.x to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3.0.x to hidden.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3220935-alter-of-config.installer to active.

🇺🇸United States trackleft2 Tucson, AZ 🇺🇸

trackleft2 changed the visibility of the branch 3220935-alter-of-config.installer to hidden.

Production build 0.71.5 2024