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

Mark Obsolete messages as error in v4.7 #736

Closed
dotnetjunkie opened this issue Aug 10, 2019 · 4 comments
Closed

Mark Obsolete messages as error in v4.7 #736

dotnetjunkie opened this issue Aug 10, 2019 · 4 comments

Comments

@dotnetjunkie
Copy link
Collaborator

@dotnetjunkie dotnetjunkie commented Aug 10, 2019

There are several classes and members that are marked with an ObsoleteAttribute. Some of them are marked with error: false while describing they will be set as error: true from version v4.7 and up.

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator Author

@dotnetjunkie dotnetjunkie commented Aug 10, 2019

feature-736 branch created.

@dotnetjunkie dotnetjunkie added this to the v4.7 milestone Aug 10, 2019
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator Author

@dotnetjunkie dotnetjunkie commented Aug 10, 2019

Also fix the messages of the following members:

  • AdvancedExtensions.GetItem
  • BehaviorDeprecationExtensions.BuildExpression

They miss a space in their message.

@singh-manvinder

This comment has been minimized.

Copy link

@singh-manvinder singh-manvinder commented Aug 30, 2019

Hi,
Should the unit tests of these obsolete methods also be removed as they causing build error because of error = true?

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator Author

@dotnetjunkie dotnetjunkie commented Sep 4, 2019

@singh-manvinder

It depends. There are basically two option:

  • Error=true on functionality that is still implemented.
  • Error=true on removed functionality, e.g. throwing a InvalidOperationException.

Sometimes, features keep implemented, even though we let the C# compiler give a compile error. This allows older integration libraries (built by others) that depend on that feature to keep working, while you allow new development to stop using that feature. This is a strategy I don't often use btw.

But as long as the feature is implemented, unit tests are warranted, although, since the feature is ready to be removed, unit tests could theoretically be removed, although that risks introducing breaking changes on code that might still be used by other libraries.

In case the feature is no longer implemented, the solution is simple: remove the unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.