Move into Shared for SqlConnectionString.cs #1329
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
…update references in the csprojs
…, IDE0090, IDE0066, IDE0018, IDE0075, and IDE0057
DavoudEshtehari
added
the
➕ Code Health
…rename of ADP.IsWindowsNT to ADP.s_isWindowsNT
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Show resolved
Hide resolved
…NYM to reference from DbConnectionStringCommon.cs
…date the functional tests where they check the exception message to also be case insensitive
…re due to case senstive exception message string comparison, and update SqlConnectionTest to use OrdinalIgnoreCase instead of CurrentCultureIgnoreCase
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Outdated
Show resolved
Hide resolved
…ist wasn't set due to a missed merge and remove unnecessary namespace
lcheunglci
force-pushed
the
MergeShared-SqlConnectionString
branch
from
5126f19
to
604d035
Compare
cheenamalhotra
requested review from
JRahnama,
johnnypham,
DavoudEshtehari and
Kaur-Parminder
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
Show resolved
Hide resolved
cheenamalhotra
approved these changes
Oct 18, 2021
JRahnama
approved these changes
Oct 18, 2021
|
Have you checked the discussion in #1265? @cheenamalhotra |
johnnypham
reviewed
Oct 18, 2021
| @@ -952,6 +952,7 @@ internal static class DbConnectionStringDefaults | |||
| internal const bool ContextConnection = false; | |||
| internal static readonly bool TransparentNetworkIPResolution = !LocalAppContextSwitches.DisableTNIRByDefault; | |||
| internal const string NetworkLibrary = ""; | |||
| internal const bool Asynchronous = false; | |||
There was a problem hiding this comment.
I don't think this is needed since Async is true by default now. But maybe we can remove it in a future PR since this one is already big.
|
@lcheunglci I think some of my reviews haven't been seen. Just brought them up here to catch your attention: |
I see, thanks, plz ignore it then! |
DavoudEshtehari
approved these changes
Oct 19, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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 issue #1261 . I merged the netfx to netcore of SqlConnectionString.cs and moved it into shared src. I did notice there was a SqlConnectionString.NetCoreApp.cs which is an empty class. I'm not sure if we should remove it so I kept it. I didn't split this up into partial classes because the differences between netfx and netcore is relatively small and to reduce redundancy of the code. During the code style clean up, I chose not to apply a few info messages namely IDE0038 i.e. code matching on line 440 because it was fairly clear and readable, IDE0057 on 843 simplifying the string substring, I tried all the suggestions that visual studio provided but was unable to remove the info message from the errorlist, and lastly, IDE0075 on line 1205, it feels reductant to check for the
Parsetablefor theAuthenticationkey returntruein both cases sinceDEFAULT.Encrpytis alsotrue.