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 symfony/flex dependency #4563

Merged
merged 12 commits into from
Dec 15, 2020
Merged

Add symfony/flex dependency #4563

merged 12 commits into from
Dec 15, 2020

Conversation

craigh
Copy link
Member

@craigh craigh commented Dec 12, 2020

This removes zikula/core dependency on symfony/symfony and instead requires needed components. Additionally, it adds dependency on symfony/flex. Flex is not useful in this state however except for core development (which seems minimal). Once this is merged, the distribution would need to be updated as well to ensure the flex dependency is used there. When this is done, real-world (downline) development starting with the distribution becomes possible using flex.

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets -
Refs tickets #4352, #3601, #3438, #3962
License MIT
Changelog updated yes

Using flex has become increasingly needed with the premier of the symfony/ux initiative which relies on webpack-encore as well. I think we could implement webpack-encore without using it to "install" our primary front-end dependencies (bootstrap, jquery, etc). but instead use it initially only for the core.css file (and maybe the jQuery config file) and then, allow it to be used for downline development.

@craigh craigh added the Feature label Dec 12, 2020
@craigh craigh added this to the 3.1.0 milestone Dec 12, 2020
@craigh craigh requested a review from Guite December 12, 2020 20:06
Guite
Guite previously requested changes Dec 12, 2020
Copy link
Member

@Guite Guite left a comment

Choose a reason for hiding this comment

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

I really like this change.
But I vote for adding all Symfony components for 3.x to ensure BC.

Background: until #3438 is done the core can not ensure that no conflicts may occur because of extensions requiring additional Symfony components (in possibly too old or too new version). Hence, extensions need to rely on the core providing the entire Symfony ecosysystem. Of course this has limits (e.g. we should not include all possible notifier packages), but the most important components need to be kept available even if the core does not use them internally. One example for this is the messenger component.

update changelog
@craigh
Copy link
Member Author

craigh commented Dec 13, 2020

@Guite, I have added the remaining components AFAIK except those noted in the updated changelog. Not noted there, these are also not added:

  • symfony/inflector (deprecated)
  • symfony/semaphore (experimental)
  • symfony/uid (experimental)

I believe semaphore and uid were added in 5.2 anyway, so there is no reason to include them. inflector is to be replaced by the string component.

I've worked hard to ensure BC. I believe I've got all as needed. Please review again.

@craigh craigh requested a review from Guite December 13, 2020 00:36
@craigh craigh dismissed Guite’s stale review December 13, 2020 00:37

concerns addressed

Copy link
Member

@Guite Guite left a comment

Choose a reason for hiding this comment

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

Nice!

Adding uid soon will become useful though, but it is not urgent 😄

@craigh
Copy link
Member Author

craigh commented Dec 15, 2020

@Guite any further thoughts or concerns here? If not, I will merge this later when I have time. Looks like I will do it locally to deal with the conflicts.

@Guite
Copy link
Member

Guite commented Dec 15, 2020

No objections.

@craigh craigh merged commit ea74c1f into master Dec 15, 2020
@craigh craigh deleted the _flex branch December 15, 2020 22:22
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.

None yet

2 participants