Skip to content
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

Add tests for collection removal without triggering validation #1737

Conversation

jordisala1991
Copy link
Member

@jordisala1991 jordisala1991 commented Apr 10, 2023

Subject

I am targeting this branch, because this is BC.

Changelog

### Changed
- Bump `sonata-project/form-extensions` to ^1.19

@jordisala1991
Copy link
Member Author

Once the PR is merged and released, this should not fail (It will require bump requirements to not fail on lower deps @VincentLanglet

@jordisala1991
Copy link
Member Author

With this we have tests for the CollectionType, case where we fail validation (not removing an item) and case where the validation is not executing because even it the collection item is not valid, it is also removed.

composer.json Outdated Show resolved Hide resolved
@jordisala1991 jordisala1991 marked this pull request as ready for review April 10, 2023 16:21
@VincentLanglet
Copy link
Member

I would consider bumping a dependency minor as pedantic, no ?

@VincentLanglet VincentLanglet merged commit 18d5bd5 into sonata-project:4.x Apr 10, 2023
@jordisala1991
Copy link
Member Author

Not sure about that TBH , first I tagged as pedantic, but since it can affect users of this package, I changed it to minor.

@VincentLanglet
Copy link
Member

Not sure about that TBH , first I tagged as pedantic, but since it can affect users of this package, I changed it to minor.

Developper doesn't need any notification since composer will do the version resolution.
And it would be better to tell dev-kit that no release is needed so far for this library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants