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

prepend Twig form_themes config #444

Merged
merged 2 commits into from Jun 16, 2023

Conversation

Hanmac
Copy link
Contributor

@Hanmac Hanmac commented Jun 14, 2023

Subject

I am targeting this branch, because is that BC or not?

Changelog

### Added
- Add datepicker twig template to the global ones

BC or not BC? or can this be done in 1.x too?

PageBundle does it too:
https://github.com/sonata-project/SonataPageBundle/blob/489e2fa48ce170b73e8bebc7ca9c10fac1d7778e/src/DependencyInjection/SonataPageExtension.php#L39

@VincentLanglet
Copy link
Member

@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 14, 2023

Does doing this allow to remove it from https://github.com/sonata-project/SonataPageBundle/blob/489e2fa48ce170b73e8bebc7ca9c10fac1d7778e/src/DependencyInjection/SonataPageExtension.php#L39 ?

Yes, also it seems that the Page one can stop implementing the Prepend Interface

Also for BC, it doesn't cause problems if both modules try to add the template

For this MR, I think it could be done in the 1.x branch because it should be BC?

@VincentLanglet
Copy link
Member

It seems BC to me ; wdyt @jordisala1991 ?

@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 14, 2023

Also the part in the Documentation needs to be updated
But that can be done in different MR?

@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 15, 2023

i think we could do, this in the 1.x branch because it should not be a BC break

then PageBundle can require that version and remove their stuff. This would be a BC break for PageBundle because it doesn't need PrependExtensionInterface anymore

@VincentLanglet
Copy link
Member

Also the part in the Documentation needs to be updated But that can be done in different MR?

It's better to do the doc changes in the same PR which is changing the behavior

@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 15, 2023

i removed the part from the Documentation about setup the twig config

VincentLanglet
VincentLanglet previously approved these changes Jun 15, 2023
@Hanmac
Copy link
Contributor Author

Hanmac commented Jun 15, 2023

i just need to know if i should rebase it against 1.x branch or not

@VincentLanglet
Copy link
Member

i just need to know if i should rebase it against 1.x branch or not

To me yes.

@Hanmac Hanmac closed this Jun 15, 2023
@Hanmac Hanmac reopened this Jun 15, 2023
@jordisala1991 jordisala1991 merged commit 30e814e into sonata-project:1.x Jun 16, 2023
18 checks passed
@jordisala1991
Copy link
Member

Thanks @Hanmac , after we deal with release of this pr, could you deal with the pagebundle code removal?

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

3 participants