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

Free up the getDescription method to prevent it overriding db fields #785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michalkleiner
Copy link
Contributor

@michalkleiner michalkleiner commented Mar 28, 2020

The getDescription method is potentially overriding a Description db field on any extending class.

The method was used only to get an addition to getTypeNice to display in the gridfield, possibly translated.

Let me know if this needs to be targetted at any other branch as it might be seen as a BC issue, should someone use this public, though otherwise quite narrow-focused, method somewhere else.

Issue

@ScopeyNZ
Copy link
Member

Unfortunately we can't break SemVer justified by the likelihood of someone (not) using the API, as much as I wish that were true. This will have to be a change targeted at master. You're welcome to do a BC PR, that deprecates the method. You can leave in the changes made to ::getTypeNice too.

@emteknetnz
Copy link
Member

This pull request hasn't had any activity for a while. Are you going to be doing further work on it, or would you prefer to close it now?

@michalkleiner
Copy link
Contributor Author

michalkleiner commented Sep 1, 2020

Yes @emteknetnz, I'll rebase and retarget the PR.

@michalkleiner michalkleiner changed the base branch from 4.3 to master September 1, 2020 22:17
@emteknetnz
Copy link
Member

@michalkleiner are you still interested in this one? I have no idea when the next major version of elemental will be released

@michalkleiner
Copy link
Contributor Author

@emteknetnz can we merge as is since it now targets master? At least that won't get lost. I may then raise a separate PR deprecating the method.

@emteknetnz
Copy link
Member

I'm OK with that, link through the deprecation PR and I'll merge this as is

@michalkleiner
Copy link
Contributor Author

@emteknetnz#953

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.

None yet

4 participants