Skip to content

[Validator] Add min and max in both error messages of LengthValidator #60805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 26, 2025

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Jun 16, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues --
License MIT

This would allow to use the same message for a Length constraint which say
"The length needs to be between {{ min }} and {{ max }}".

@carsonbot carsonbot added this to the 7.4 milestone Jun 16, 2025
@OskarStark OskarStark changed the title [Validator] Add min and max in both error message of LengthValidator [Validator] Add min and max in both error messages of LengthValidator Jun 16, 2025
@VincentLanglet
Copy link
Contributor Author

The Tests on 8.2 high-deps are failing because this PR
fc7d413
changes the tests and the CI is trying to run on the branch 6.4
image

How do I solve this ?

@VincentLanglet VincentLanglet force-pushed the lengthValidator branch 2 times, most recently from f51245e to 8df3197 Compare June 20, 2025 21:14
@Spomky
Copy link
Contributor

Spomky commented Jul 17, 2025

Looks good.

I noticed some failures are related to these changes.
Would you mind to rebase (solve the conflicts) and fix the tests?
Regards

@VincentLanglet
Copy link
Contributor Author

I noticed some failures are related to these changes. Would you ming to rebase (solve the conflicts) and fix the tests? Regards

Hi ; I rebased but I dunno how to fix the test.

See https://github.com/symfony/symfony/actions/runs/16356617489/job/46216379960?pr=60805

The Unit test 8.2 high-deps is checking out the code on Symfony 6.4

Checking out Symfony 6.4 and running tests with patched components as deps

So this file changes are lost
https://github.com/symfony/symfony/pull/60805/files#diff-927362e8296b98f998ae0368419317fd75a34054f520cc354c691ddc22cb5b0c

And of course tests are failing since they don't have the {{ min }} value I added then...

Any clue how to fix this @Spomky ?

@VincentLanglet
Copy link
Contributor Author

Friendly ping @nicolas-grekas you might know how to solve the failure #60805 (comment)

Test are failing but I feel like I cannot fix them since it runs the 6.4 version and I'm on the 7.4 branch...

@xabbuh
Copy link
Member

xabbuh commented Jul 25, 2025

I suggest that we update the ApiAttributesTest in the 6.4 after this PR is merged and relax the assertion there.

@fabpot fabpot force-pushed the lengthValidator branch from 9ec2a5e to c465978 Compare July 26, 2025 12:32
@fabpot
Copy link
Member

fabpot commented Jul 26, 2025

Thank you @VincentLanglet.

@fabpot fabpot merged commit 2663b3b into symfony:7.4 Jul 26, 2025
9 of 11 checks passed
nicolas-grekas added a commit that referenced this pull request Jul 30, 2025
…lity with Symfony 7.4 (xabbuh)

This PR was merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle] relax assertions for forward compatibility with Symfony 7.4

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | see #60805 (comment) and my response to it
| License       | MIT

Commits
-------

0e9136e relax assertions for forward compatibility with Symfony 7.4
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.

6 participants