Move to Shared for SqlDependency.cs #1299
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r error due to variable name changes
|
I much prefer usings with scope. If the MS team agree I think we'd need to update the repo editor config to make it stop suggesting the implicitly scoped version. |
DavoudEshtehari
added
the
➕ Code Health
I have no preference as long as it's readable. |
johnnypham
reviewed
Oct 1, 2021
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs
Outdated
Show resolved
Hide resolved
johnnypham
approved these changes
Oct 1, 2021
|
@lcheunglci can you address the conflict please? |
|
@lcheunglci There are some methods that can use body expressions, for lines 134, 183, 219, 465, 561, 576, 582, 734, 740 |
Kaur-Parminder
approved these changes
Oct 13, 2021
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to #1261 . I merged the netfx version of SqlDependency into the netcore version and move it to the shared src. I was originally going to allow the resource attributes but that would require an update to the StringsHelper which is currently being merged in #1288 so I left it with the ifdef NETFRAMEWORK, but once that PR is merged, I could revised this. Also I updated the file to conform the coding style but I left two IDE0063 info messages because I found the using statement in line 512 and 545 more readable, but the intellisense recommended change is with C# 8 syntax without the extra scope body, so if that is preferred I am more than happy to change it.