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

Re-allow SF DI>4.4.0 & upgrade di test package #1105

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Re-allow SF DI>4.4.0 & upgrade di test package #1105

merged 1 commit into from
Jan 27, 2020

Conversation

mazsudo
Copy link
Contributor

@mazsudo mazsudo commented Jan 24, 2020

Subject

I am targeting this branch, because it does not contain BC break i guess.

to understand this:

Changelog

### Changed
- Re-allow SF DI>4.4.0
- Upgrade matthiasnoback/symfony-dependency-injection-test to ^4.0

mazsudo referenced this pull request Jan 24, 2020
Looks like there is something incompatible with
sonata-project/page-bundle in that component, because blacklisting
version 4.4 fixes the build.
@greg0ire greg0ire added the minor label Jan 24, 2020
@greg0ire
Copy link
Contributor

If you want to use the latest di test package, you will have to upgrade phpunit to version 8 (can be done in .travis.yml. If that works, we will have to do a dev-kit PR.

core23
core23 previously approved these changes Jan 24, 2020
@greg0ire
Copy link
Contributor

You will have to add :void to all setUp and tearDown methods if you want this to work.

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 24, 2020

not sure but according to the build i guess i need to open a PR on CoreBundle

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 24, 2020

waiting for sonata-project/SonataCoreBundle#729 to be merged :)

@core23
Copy link
Member

core23 commented Jan 26, 2020

waiting for sonata-project/SonataCoreBundle#729 to be merged :)

The CoreBundle is deprecated, please try to port the breaking code to this bundle, otherwise we must do this, when we drop the CoreBundle (e.g. on the master branch)

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 26, 2020

@core23 don't think there is any breaking code to port here. Build is just failing because of CoreBundle or am I miss understanding?

@greg0ire
Copy link
Contributor

@core23 is saying you should just copy the XliffValidatorTestCase to the page bundle if I understand correctly.

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 27, 2020

almost understood. However, even with migrating all tests from CoreBundle to PageBundle (because some others will make the build fail), without merging anything in CoreBundle it will still fail. Or do we need to merge the removal of all test inside CoreBundle first? Not sure of what i have to do here sorry 😬

@greg0ire
Copy link
Contributor

XliffTest should no longer extend the class from the core bundle I think. You can copy all necesseary method from XliffValidatorTestCase to it. That will remove the dependency to the core bundle for tests.

because some others will make the build fail

Some other tests? Which ones?

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 27, 2020

sorry i miss understood the core bundle relationship in the test 😬 hope i did what was expected ;)

Copy link
Contributor

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

You did terrific 👍 let's merge

@greg0ire greg0ire merged commit cd23a18 into sonata-project:3.x Jan 27, 2020
@greg0ire
Copy link
Contributor

I created a PR on dev-kit to make sure we stay with PHPUnit 8: sonata-project/dev-kit#541

Also, I'm going to release a new version of the page bundle

@mazsudo mazsudo deleted the test-di-from-3-14-0 branch January 27, 2020 19:35
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.

3 participants