Lib updates and prepare for Microsoft.Data.SqlClient #1313
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
|
Suggest look at dotnet/SqlClient#30 (comment) |
mgravell
changed the title
…an via a hard-coded handler (AddSqlDataRecordsTypeHandler)
…eaking change, so rev the semver
… which Microsoft.Data tests *will never work*
NickCraver
reviewed
Aug 27, 2019
NickCraver
reviewed
Aug 27, 2019
| </dependentAssembly> | ||
| </assemblyBinding> | ||
| </runtime> | ||
| </configuration> |
There was a problem hiding this comment.
Should be able to delete this app.config and it generate as the <name>.config file in the output directory
There was a problem hiding this comment.
nope, doesn't generate the binding for Microsoft.SqlServer.Types if you do that; is needed
NickCraver
reviewed
Aug 27, 2019
NickCraver
reviewed
Aug 27, 2019
| @@ -20,6 +20,6 @@ | |||
| <Reference Include="Microsoft.CSharp" /> | |||
| </ItemGroup> | |||
| <ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.3'"> | |||
| <PackageReference Include="Microsoft.CSharp" Version="4.3.0" /> | |||
| <PackageReference Include="Microsoft.CSharp" Version="4.5.0" /> | |||
There was a problem hiding this comment.
Let's drop netstandard1.3 from all builds
NickCraver
reviewed
Aug 27, 2019
NickCraver
reviewed
Aug 27, 2019
Dapper.Tests/Dapper.Tests.csproj
Outdated
| </ItemGroup> | ||
| <ItemGroup Condition="'$(Platform)'=='x86'"> | ||
| <Content Include="$(USERPROFILE)\.nuget\packages\microsoft.sqlserver.types\14.0.1016.290\nativeBinaries\x86\*.dll" | ||
| Condition="'$(TargetFramework)' == 'net46' OR '$(TargetFramework)' == 'net472'"> |
There was a problem hiding this comment.
Let's move these conditions to the ItemGroup (same above)
NickCraver
reviewed
Aug 27, 2019
Dapper.Tests/Dapper.Tests.csproj
Outdated
| <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.0'"> | ||
| <PackageReference Include="Microsoft.Data.Sqlite" Version="2.0.0" /> | ||
| <PackageReference Include="Microsoft.Data.Sqlite" Version="2.2.6" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Can drop this block with netcoreapp2.0 build drop
NickCraver
reviewed
Aug 27, 2019
| } | ||
| finally | ||
| { | ||
| connection.Execute("DROP TYPE int_list_type"); | ||
| try { connection.Execute("DROP TYPE int_list_type"); } catch { } |
There was a problem hiding this comment.
Let's double check if we need these with the collections defined now
NickCraver
reviewed
Aug 27, 2019
Dapper.Tests/ParameterTests.cs
Outdated
| @@ -686,6 +741,8 @@ public void DBGeography_SO24405645_SO24402424() | |||
| [Fact] | |||
| public void SqlGeography_SO25538154() | |||
| { | |||
| if (IsMsDataClient) return; // not supported | |||
There was a problem hiding this comment.
NickCraver
reviewed
Aug 27, 2019
# Conflicts: # Dapper/SqlMapper.cs
…overhead, subclass Identity so we don't always need the arrays
NickCraver
changed the title
rhubley
pushed a commit
to rhubley/Dapper
that referenced
this pull request
Jan 29, 2020
…1313) v2 work; primarily prep for split SqlClient
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.
Intent: make Dapper usable with both System.Data.SqlClient and Microsoft.Data.SqlClient
Details:
IEnumerable<System.Data.SqlDataRecord>toIEnumerable<T> where T : IDataRecord; this makes it a breaking change - although one that is very unlikely to impact many people at all, so:Note: the removal of System.Data.SqlClient may impact consumers who depended on Dapper to get that ref, but I strongly consider removing it a GoodThing™ for all; folks can add the correct package now. Will need a release note.
Re the semver bump; my view here is: