Skip to content

Make obsolete abstract virtual #1770

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

Merged

Conversation

fredericDelaporte
Copy link
Member

I have realized than obsoleting an abstract member is not very nice to implementors: it forces them to override an obsolete member. Once dropped, their code will no more compile, while the purpose of Obsolete is to allow consumers to make required changes for next version and avoid errors due to upgrading.

Instead, we should switch such abstract member to virtual.

This is a kind of follow-up to #1599, since the only obsoleted abstract I have found was introduced there. (I have thought of this subject because I was starting to obsolete abstract members for another subject.)

There is somewhat a similar trouble with obsoleted interface members, but there is nothing we can do for them, and furthermore the implementor can implements them in a way that will not break compilation once they are dropped, contrary to abstract members.

@bahusoid
Copy link
Member

bahusoid commented Jun 23, 2018

My only concern is method "discoverability" for new implementors. Yes - you are forced to implement obsolete method but you also see a proper new method to override in obsolete message. With this method virtual it's not clear what should be overridden in the first place.

If you think it's OK - no objections from my side. Code is definitely cleaner this way.

@fredericDelaporte
Copy link
Member Author

They will still have an exception if they do some minimal testing of their implementation. Of course, it is better to know that at compilation time, but that is the reason why the non obsoleted method is flagged for conversion to abstract in 6.0.
I do not think using an obsolete abstract to hint at the virtual to override is adequate for "compensating" during the "transition period", due to the drawback it has of not allowing implementors to actually get their code ready for the next major version.

@fredericDelaporte fredericDelaporte merged commit 8030961 into nhibernate:master Jun 24, 2018
@fredericDelaporte fredericDelaporte deleted the AbstactObsolete branch June 24, 2018 15:44
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.

3 participants