-
Notifications
You must be signed in to change notification settings - Fork 244
Obsolete old APIs #605
Obsolete old APIs #605
Conversation
|
Awesome, it's great to see this! Small nitpick: There's a Visual Studio Version downgrade in the sln file. |
|
|
||
| /// <summary> | ||
| /// <para> | ||
| /// This method is being removed from the ILoggerFactory abstraction, to have similar behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend following a pattern closer to the guidelines: https://github.com/aspnet/Home/wiki/Engineering-guidelines#breaking-changes
| /// Adds an <see cref="ILoggerProvider"/> to the logging system. | ||
| /// </summary> | ||
| /// <param name="provider">The <see cref="ILoggerProvider"/>.</param> | ||
| [Obsolete("This method is being removed from the ILoggerFactory abstraction, to have similar behavior" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
|
|
||
| /// <summary> | ||
| /// <para> | ||
| /// This method is obsolete, use AddProvider on the concrete Microsoft.Extensions.Logging.LoggerFactory class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we follow the guidelines on Recommend following a pattern closer to the guidelines: https://github.com/aspnet/Home/wiki/Engineering-guidelines#breaking-changes a little more closely? I recommend using the exact phrasing in there, i.e. This method is obsolete and will be removed in a future version. The recommended alternative is Microsoft.Extensions.Logging.LoggerFactory.AddProvider().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Applies to all the obsoletions in this PR.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is obsolete and will be removed in a future version. The recommended alternative is Microsoft.Extensions.Logging.AddConsole() on Microsoft.Extensions.Logging.LoggerFactory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is obsolete and will be removed in a future version. The recommended alternative is to call the Microsoft.Extensions.Logging.AddConsole() extension method on the Microsoft.Extensions.Logging.LoggerFactory instance.
Eilon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Can you log a new bug that says to actually delete all these obsolete methods in a future version?
0f5c64b to
dc151a1
Compare
dc151a1 to
e148579
Compare
No description provided.