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.

Sign up for GitHub

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

Jump to bottom

Fix | SqlBuffer.SqlGuid #2310

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Fix | SqlBuffer.SqlGuid #2310

merged 3 commits into from
Apr 9, 2024

Conversation

wilbit
Copy link
Contributor

@wilbit wilbit commented Jan 21, 2024 •
edited

Bug description:
SqlBuffer.get_SqlGuid could return SqlGuid with incorrect (or empty) Guid value inside after SqlBuffer.SetToNullOfType(StorageType.Guid) is invoked.

@wilbit wilbit mentioned this pull request Jan 21, 2024
Merged
@wilbit wilbit changed the title Fix & Perf | SqlBuffer Fix & Perf | SqlBuffer.SqlGuid Jan 21, 2024
@wilbit wilbit marked this pull request as ready for review January 22, 2024 00:10
@DavoudEshtehari DavoudEshtehari added this to the 5.2.0-preview5 milestone Jan 22, 2024
@DavoudEshtehari DavoudEshtehari added the 🐛 Bug! Something isn't right ! label Jan 22, 2024
@DavoudEshtehari DavoudEshtehari removed this from the 5.2.0-preview5 milestone Jan 23, 2024
@wilbit wilbit changed the title Fix & Perf | SqlBuffer.SqlGuid WIP: Fix & Perf | SqlBuffer.SqlGuid Jan 23, 2024
@wilbit
Copy link
Contributor Author

wilbit commented Jan 23, 2024 •
edited

Because the revert PR #2311 is merged, I will replace this PR's commits with functional tests for SqlBuffer and a bugfix

@DavoudEshtehari DavoudEshtehari marked this pull request as draft January 24, 2024 00:30
wilbit added a commit to Intrigma/SqlClient that referenced this pull request Jan 24, 2024
@wilbit
Test | SqlBuffer.Guid and SqlBuffer.SqlGuid tests (dotnet#2310)
6b67060
wilbit added a commit to Intrigma/SqlClient that referenced this pull request Jan 24, 2024
@wilbit
Fix | SqlBuffer.get_SqlGuid could return incorrect Guid (dotnet#2310)
ebf0c88 
after SetToNullOfType(StorageType.Guid) is invoked
@wilbit wilbit changed the title WIP: Fix & Perf | SqlBuffer.SqlGuid Fix | SqlBuffer.SqlGuid Jan 24, 2024
@codecov Codecov
Copy link

codecov bot commented Jan 25, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c8b70f1) 72.47% compared to head (be27257) 72.61%.
Report is 5 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2310      +/-   ##
==========================================
+ Coverage   72.47%   72.61%   +0.13%     
==========================================
  Files         310      310              
  Lines       61873    61877       +4     
==========================================
+ Hits        44845    44930      +85     
+ Misses      17028    16947      -81
Flag Coverage Δ
addons 92.88% <ø> (ø)
netcore 76.74% <100.00%> (+0.07%) ⬆️
netfx 70.24% <100.00%> (+0.29%) ⬆️

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

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

@wilbit wilbit marked this pull request as ready for review January 25, 2024 07:26
@wilbit
Copy link
Contributor Author

wilbit commented Jan 25, 2024

Hmm, I don't get what it actually failed in the tests =\

@DavoudEshtehari
Copy link
Member

Hmm, I don't get what it actually failed in the tests =\

@wilbit No need to worry; this seems to be a random Azure resource issue.

@wilbit wilbit requested a review from Wraith2 January 26, 2024 10:05
@wilbit
Copy link
Contributor Author

wilbit commented Jan 30, 2024

Any feedback about the PR?

@wilbit wilbit requested a review from roji February 7, 2024 12:08
@wilbit
Copy link
Contributor Author

wilbit commented Feb 14, 2024

Anything?

@JRahnama
Copy link
Member

Anything?

This will be reviewed for the next preview release. We are on code freeze now for the GA release.

Wraith2
Wraith2 approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@Wraith2 Wraith2 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, real life stuff happened.

@kf-gonzalez kf-gonzalez added this to the 6.0-preview1 milestone Feb 21, 2024
@wilbit
Copy link
Contributor Author

wilbit commented Apr 9, 2024

May we merge this PR?
I have other changes (improvements) for SqlBuffer, but they conflict with this PR, so it does not make sense to submit them =\

@DavoudEshtehari DavoudEshtehari merged commit f5df519 into dotnet:main Apr 9, 2024
148 checks passed
@DavoudEshtehari DavoudEshtehari mentioned this pull request Apr 9, 2024
Merged
@wilbit wilbit deleted the #2300-SqlBuffer branch April 9, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wraith2 Wraith2 Wraith2 approved these changes

@David-Engel David-Engel David-Engel approved these changes

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

@JRahnama JRahnama Awaiting requested review from JRahnama

@roji roji Awaiting requested review from roji

Assignees
No one assigned
Labels
🐛 Bug! Something isn't right !
Projects
None yet
Milestone
6.0-preview1
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@wilbit @DavoudEshtehari @JRahnama @roji @Wraith2 @David-Engel @kf-gonzalez