Skip to content

Conversation

@OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Aug 6, 2025

Q A
Bug fix? no
New feature? yes
Docs? no
Issues --
License MIT
  • Update all composer.json files to require Symfony ^7.3|^8.0
  • Minimum supported version is now Symfony 7.3 LTS

Needs

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Thanks for rolling that out - there should also be some code that I annotated with 6.4 comments - but can easily remove that in a follow up

@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch 2 times, most recently from fb2605a to 6080ad7 Compare August 6, 2025 21:23
@Nyholm
Copy link
Member

Nyholm commented Aug 6, 2025

Why do we want to drop 6.4?

I don't think there is a big burden to support it and it it a huge benefit for adoption of we keep it.

Or am I missing something?

@fabpot
Copy link
Member

fabpot commented Aug 7, 2025

Why do we want to drop 6.4?

I don't think there is a big burden to support it and it it a huge benefit for adoption of we keep it.

Or am I missing something?

If you're not able to upgrade to a non-LTS version of Symfony, you will probably not be able to use Symfony AI, which is not stable yet (and probably not before the release of 7.4 LTS), you will probably not be able to upgrade Symony AI frequently which is going to be needed as the space is changing fast.

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

the CI failures look related

@OskarStark
Copy link
Contributor Author

the CI failures look related

yes, but I think this is due to the fact that not all Symfony components allow 7.4|8 ? just my guess

@derrabus
Copy link
Member

derrabus commented Aug 7, 2025

the CI failures look related

yes, but I think this is due to the fact that not all Symfony components allow 7.4|8 ? just my guess

No, it's because you're pinning to unstable Symfony branches while requesting stable dependencies.

@OskarStark
Copy link
Contributor Author

Ok, so we can only merge this, once 7.4 and 8.0 are released?

@derrabus
Copy link
Member

derrabus commented Aug 7, 2025

No, you'll have to lower the minimum stability to dev. Either globally for the project (as we do on symfony/symfony) or on those two CI jobs.

@valtzu
Copy link
Contributor

valtzu commented Aug 8, 2025

👎 from me (at least until 7.4 is out). This will change a one-week proof-of-concept to a bigger "upgrade symfony" project for many companies – assuming that companies prefer LTS (yet are fine running symfony/ai@dev).

@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch from b332a57 to 29e1a88 Compare August 13, 2025 12:25
@carsonbot carsonbot changed the title Drop Symfony 6.4 support, require ^7.3|^8.0 Drop Symfony 6.4 support, require ^7.3|^8.0 Aug 13, 2025
@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch from 29e1a88 to d045c2c Compare August 20, 2025 11:19
@OskarStark
Copy link
Contributor Author

@derrabus I now use the proposed minimum stability and the 7.4 build is passing, but the 8.0 one not 🤔

Loading composer repositories with package information
Restricting packages listed in "symfony/symfony" to "8.0.*"
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires symfony/finder ^7.3|^8.0 -> satisfiable by symfony/finder[8.0.x-dev].
    - symfony/finder 8.0.x-dev requires php >=8.4 -> your php version (8.3.23) does not satisfy that requirement.
  Problem 2
    - Root composer.json requires symfony/filesystem ^7.3|^8.0 -> satisfiable by symfony/filesystem[8.0.x-dev].
    - symfony/filesystem 8.0.x-dev requires php >=8.4 -> your php version (8.3.23) does not satisfy that requirement.

Error: Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires symfony/finder ^7.3|^8.0 -> satisfiable by symfony/finder[8.0.x-dev].
    - symfony/finder 8.0.x-dev requires php >=8.4 -> your php version (8.3.23) does not satisfy that requirement.
  Problem 2
    - Root composer.json requires symfony/filesystem ^7.3|^8.0 -> satisfiable by symfony/filesystem[8.0.x-dev].
    - symfony/filesystem 8.0.x-dev requires php >=8.4 -> your php version (8.3.23) does not satisfy that requirement.

@OskarStark OskarStark requested review from derrabus and xabbuh August 20, 2025 11:42
@derrabus
Copy link
Member

@OskarStark Read the error message. ✌️ You need to run the Symfony 8 workflow on PHP 8.4.

@OskarStark
Copy link
Contributor Author

Thought I pushed it already

@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch from 5b8a29f to 7ce03af Compare September 3, 2025 08:20
CodeWithKyrian added a commit to CodeWithKyrian/transformers-php that referenced this pull request Sep 15, 2025
### What:

- [ ] Bug Fix
- [x] New Feature

### Description:

allow the package to be used with version 8 of Symfony components

### Related:

Widening the accepted releases of the Symfony Console component will
allow us to already test the Symfony AI packages with the upcoming
Symfony 8 major release (see symfony/ai#277)
@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch from 7ce03af to e75995b Compare September 15, 2025 09:37
@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch 2 times, most recently from c5e91f1 to c0d7d52 Compare September 15, 2025 20:13
@OskarStark
Copy link
Contributor Author

OskarStark commented Sep 15, 2025

@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch 3 times, most recently from cd6b1c4 to 9574f3a Compare September 17, 2025 11:39
OskarStark added a commit that referenced this pull request Sep 17, 2025
…` (OskarStark)

This PR was merged into the main branch.

Discussion
----------

[Examples] Add `minimum-stability` `dev` to `composer.json`

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Docs?         | no
| Issues        | --
| License       | MIT

Adds minimum-stability configuration to allow dev dependencies and development versions of packages in examples.

Needed for #277, thanks `@xabbuh`

Commits
-------

f352744 [Examples] Add minimum-stability dev to composer.json
@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch 2 times, most recently from 6ba655c to d7f9a2b Compare September 17, 2025 12:27
@OskarStark OskarStark force-pushed the drop-symfony-6.4-support branch from d7f9a2b to b9dd575 Compare September 17, 2025 12:30
@OskarStark OskarStark merged commit 264b4e3 into symfony:main Sep 17, 2025
13 checks passed
OskarStark added a commit that referenced this pull request Sep 17, 2025
…tring configuration values (OskarStark)

This PR was squashed before being merged into the main branch.

Discussion
----------

[AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| Docs?         | no
| Issues        | --
| License       | MIT

Replace `scalarNode()` with more specific node types for better configuration type validation:

- Use `stringNode()` for string values (API keys, URLs, service names, etc.)
- Use `integerNode()` for integer values (dimensions, etc.)
- Keep `scalarNode()` only for mixed-type fields where appropriate

This provides better type validation and makes the configuration more self-documenting.

## Changes Made

- **String fields**: API keys, URLs, endpoints, service names, class names, database/collection names, etc. now use `stringNode()`
- **Integer fields**: `dimensions` and `top_k` fields now use `integerNode()`
- **Mixed fields**: Strategy, metric, and similar fields remain as `scalarNode()`

## Needs
- [ ] #277

Commits
-------

492f75f [AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants