Skip to content

C#: Migrate SQL sinks to CSV format #6196

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

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

tamasvajk
Copy link
Contributor

@tamasvajk tamasvajk commented Jul 1, 2021

This PR migrates some SQL sinks (known IDbCommand implementors, EntityFramework, Dapper) to CSV format.

Diff job

Some required fixes are added here:

@github-actions github-actions bot added the C# label Jul 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2021

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",3,13,5,5
+    System,"``System.*``, ``System``",3,13,28,5
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``MySql.Data.MySqlClient``",,,130,
-    Totals,,3,13,5,5
+    Totals,,3,13,158,5
  • Changes to framework-coverage-csharp.csv:
- package,sink,source,summary,sink:html,sink:xss,source:local,summary:taint
+ package,sink,source,summary,sink:html,sink:sql,sink:xss,source:local,summary:taint
+ Dapper,54,,,,54,,,
+ Microsoft.ApplicationBlocks.Data,28,,,,28,,,
+ MySql.Data.MySqlClient,48,,,,48,,,
- System,5,3,13,4,1,3,13
+ System,28,3,13,4,23,1,3,13

@tamasvajk tamasvajk force-pushed the feature/sql-sinks branch 2 times, most recently from 3203e42 to fdf3cf4 Compare July 1, 2021 14:08
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2021

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",3,13,5,5
+    System,"``System.*``, ``System``",3,13,28,5
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``MySql.Data.MySqlClient``",,,119,
-    Totals,,3,13,5,5
+    Totals,,3,13,147,5
  • Changes to framework-coverage-csharp.csv:
- package,sink,source,summary,sink:html,sink:xss,source:local,summary:taint
+ package,sink,source,summary,sink:html,sink:sql,sink:xss,source:local,summary:taint
+ Dapper,43,,,,43,,,
+ Microsoft.ApplicationBlocks.Data,28,,,,28,,,
+ MySql.Data.MySqlClient,48,,,,48,,,
- System,5,3,13,4,1,3,13
+ System,28,3,13,4,23,1,3,13

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",3,13,5,5
+    System,"``System.*``, ``System``",3,13,28,5
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``MySql.Data.MySqlClient``",,,131,
-    Totals,,3,13,5,5
+    Totals,,3,13,159,5
  • Changes to framework-coverage-csharp.csv:
- package,sink,source,summary,sink:html,sink:xss,source:local,summary:taint
+ package,sink,source,summary,sink:html,sink:sql,sink:xss,source:local,summary:taint
+ Dapper,55,,,,55,,,
+ Microsoft.ApplicationBlocks.Data,28,,,,28,,,
+ MySql.Data.MySqlClient,48,,,,48,,,
- System,5,3,13,4,1,3,13
+ System,28,3,13,4,23,1,3,13

@tamasvajk tamasvajk added the no-change-note-required This PR does not need a change note label Sep 7, 2021
@tamasvajk tamasvajk marked this pull request as ready for review September 8, 2021 13:08
@tamasvajk tamasvajk requested a review from a team as a code owner September 8, 2021 13:08
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

hvitved
hvitved previously approved these changes Sep 16, 2021
@tamasvajk
Copy link
Contributor Author

@hvitved Thanks for the approval. I've rebased this PR to fix merge conflicts after the ServiceStack PR got merged.

@tamasvajk tamasvajk merged commit 3247794 into github:main Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# documentation no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants