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

Remove redundant method override #5235

Closed

Conversation

duncan3dc
Copy link
Contributor

Q A
Type improvement
BC Break no

Summary

While investigating an issue I noticed this method overrides one that is identical, so this PR just removes the duplicate.

public function getClobTypeDeclarationSQL(array $column)
{
return 'VARCHAR(MAX)';
}

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

The change looks correct: the parent method does exactly the same. Not sure if it's worth merging this kind of change to 2.13.x though. On 3.3.x, this issue is fixed already.

@greg0ire
Copy link
Member

greg0ire commented Feb 2, 2022

It does not fall under any of the categories described in https://www.doctrine-project.org/2022/01/22/sunsetting-dbal-2.html, so let's close, sorry.

@greg0ire greg0ire closed this Feb 2, 2022
@duncan3dc
Copy link
Contributor Author

It was cleanup in preparation for a bug fix (#5237) would you rather that bugfix was more complicated and updated two duplicate methods than merge this PR?

@derrabus
Copy link
Member

derrabus commented Feb 3, 2022

We try to change as little as possible on the 2.13 branch. If a change does not fix a critical bug, we usually don't merge it anymore, especially if the problem was solved on a higher branch already.

@duncan3dc
Copy link
Contributor Author

I don't understand the relevance of that sorry, are you saying you're rejecting the unicode/nvarchar bug fix for 2.13?

@derrabus
Copy link
Member

derrabus commented Feb 3, 2022

No, I'm saying we're rejecting the change suggested in this PR.

@duncan3dc
Copy link
Contributor Author

@greg0ire already said that, so then I asked if you'd rather the bugfix this is for (#5237) was more complicated, I'll presume that's a yes...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants