Skip to content
This repository has been archived by the owner on Jul 28, 2022. It is now read-only.

Remove unused code #157

Closed
wants to merge 1 commit into from
Closed

Remove unused code #157

wants to merge 1 commit into from

Conversation

franmomu
Copy link
Member

Subject

I was going to fix the config deprecation in this bundle, but it looks like neither the Configuration class nor the Extension class are used, there are no services declared, so I guess it is better to just remove them.

I am targeting this branch, because is the one.

Changelog

### Removed
- Removed useless `Configuration` and `SonataDatagridExtension` class

Copy link
Member

@core23 core23 left a comment

Choose a reason for hiding this comment

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

We can't do this on a stable branch

@OskarStark
Copy link
Member

Yep it would be a BC break, this is not possible, please provide the patch for the deprecation here too.
Thanks

@OskarStark OskarStark closed this Dec 17, 2018
@franmomu
Copy link
Member Author

Ok, I will. But why is this BC? I thought it wouldn't because no configuration has been used in this bundle since the start, I seem to remember that DependencyInjection folder was mandatory for a bundle at the beginning of SF2, just asking to learn.

@OskarStark
Copy link
Member

One could extend the class as it is not final

@greg0ire
Copy link

Yeah that's highly improbable though… but let's not take any chances, we don't know what crazy things people are up to.

@franmomu franmomu deleted the remove_unused_code branch December 17, 2018 13:43
@franmomu
Copy link
Member Author

Makes sense, I thought the same that no one would extend those classes, but you are right, better safe than sorry.

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

Successfully merging this pull request may close these issues.

None yet

4 participants