Skip to content

Conversation

@donquixote
Copy link
Owner

@donquixote donquixote commented Jul 23, 2025

@donquixote donquixote changed the title Upgrade symfony finder (fork) Support symfony/finder ^7.1 alongside ~6.4.0 (fork) Jul 23, 2025
include:
- operating-system: 'ubuntu-latest'
php-versions: '8.1'
symfony: '~6.4.0'
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this full version range is a bit overkill, but it confirms that we can use ^7.1 rather than ^7.3 in composer.json.
In fact ^7.0 would also work, but Drupal only ever needs ^7.1 starting at drupal/core:11.0.0.

composer.json Outdated
"simplesamlphp/assert": "~1.8.1",
"simplesamlphp/composer-xmlprovider-installer": "~1.0.2",
"symfony/finder": "~6.4.0"
"symfony/finder": "~6.4.0|^7.1"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using double pipes?

Suggested change
"symfony/finder": "~6.4.0|^7.1"
"symfony/finder": "~6.4.0 || ^7.1"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing is, what is the minimum version?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I did not remember what is the preferred way to do it. Going to change it to ||.

operating-system: [ubuntu-latest]
php-versions: ['8.1', '8.2', '8.3', '8.4']
php-versions: ['8.2', '8.3', '8.4']
symfony: ['~6.4.0', '~7.1.0', '~7.2.0', '^7.3']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should test all minors available or just the same as composer.
For me it's just the latest or min-latest.
In the future if new minors are released you have to manually add them, and the CI takes more and more time.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we reduce it to ^6.4, ~7.1.0 and ^7.3, just to be sure that our min version is ok.
Keep in mind ^7.3 one will always take the last from 7.x, so that could be 7.4.* or 7.5.* in the future.

@donquixote donquixote force-pushed the fork/upgrade-symfony-finder branch from 245efb2 to dcd20f9 Compare July 24, 2025 14:32
@donquixote donquixote force-pushed the fork/upgrade-symfony-finder branch from dcd20f9 to b79fe59 Compare July 24, 2025 15:44
@donquixote donquixote merged commit 1d7c5c5 into master Jul 29, 2025
79 checks passed
@donquixote
Copy link
Owner Author

That's great!
Now I did merge this PR even though we don't really care what we have in master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants