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

Ignore Symfony 5 bumps for now #863

Merged
merged 1 commit into from
Oct 20, 2021
Merged

Conversation

ChristophWurst
Copy link
Member

Dependabot shall send us 4.x updates instead.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

Make sense but I wonder how we handle api break in our 3rdparty lib that are exposed to 3rd party apps in general. We need at some point to update to symfony 5 once symfony 4 is eol

@ChristophWurst
Copy link
Member Author

Make sense but I wonder how we handle api break in our 3rdparty lib that are exposed to 3rd party apps in general. We need at some point to update to symfony 5 once symfony 4 is eol

Excellent question. We had the same dilemma with dbal. Strictly speaking apps should never depend on any external dependencies of Nextcloud. In reality they do. For dbal we still have some public APIs that leak dbal classes. For Symfony the public API doesn't leak much, except for the already deprecated event dispatcher.

However, what will potentially blow up is all the CLI commands that directly use Symfony. For those we have to define a proper abstract public API without any direct ties on Symfony. Then apps can migrate to this and we can swap the Symfony version without too much hassle.

@skjnldsv could you possibly put this onto the todo for 24? We need a solid CLI command API to clean this up.

@ChristophWurst ChristophWurst merged commit 95a4911 into master Oct 20, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/ignore-symfony-5 branch October 20, 2021 16:48
@skjnldsv skjnldsv mentioned this pull request Oct 25, 2021
25 tasks
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.

2 participants