Skip to content
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

Merge TDSParserStateObject.StateSnapshot #2122

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Aug 13, 2023

Contributes to merging netfx and netcore codebases. I need this particular set of classes synchronized for async string perf work.

I've renamed the TdsParserStateObjects because when you have files with the name name (different paths) visual studio doesn't work very well with them. In netfx i could see both but couldn't navigate to items in them properly. In netcore only one file was visible so you couldn't get to shared parts at all. It's now clearer both in the vs ui and in PR's where we work by filename.

I've taken elements which are identical in both netfx and netcore and moved them to the shared partial class. The other elements that are not currently shared will be updated in other PR's

@David-Engel once the current release is done could this get a quick review so i can get some momentum on this merging. I can keep working on relatively small and hopefully easy to review PR's bringing netfx in sync with netcore piece by piece if you can keep the review side moving.

@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Patch coverage: 25.00% and project coverage change: +0.96% 🎉

Comparison is base (b82b1eb) 69.98% compared to head (74746dd) 70.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2122      +/-   ##
==========================================
+ Coverage   69.98%   70.95%   +0.96%     
==========================================
  Files         306      306              
  Lines       62051    61812     -239     
==========================================
+ Hits        43428    43857     +429     
+ Misses      18623    17955     -668     
Flag Coverage Δ
addons 92.88% <ø> (ø)
netcore 73.45% <25.00%> (-0.03%) ⬇️
netfx 69.69% <25.00%> (+1.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...oft/Data/SqlClient/TdsParserStateObject.netcore.cs 80.57% <ø> (ø)
...osoft/Data/SqlClient/TdsParserStateObject.netfx.cs 82.21% <ø> (ø)
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 82.02% <25.00%> (-3.05%) ⬇️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

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

Pretty straightforward changes. Thanks!

@David-Engel David-Engel added this to the 5.2.0-preview4 milestone Aug 14, 2023
@David-Engel David-Engel merged commit 3202a3c into dotnet:main Aug 16, 2023
132 checks passed
@Wraith2 Wraith2 deleted the combine-27 branch March 17, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants