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

Optimize the SqlGuid struct #72549

Merged
merged 15 commits into from
Jan 30, 2023
Merged

Optimize the SqlGuid struct #72549

merged 15 commits into from
Jan 30, 2023

Conversation

MichalPetryka
Copy link
Contributor

Optimizes the SqlGuid struct according to the idea
from #51836.

@MichalPetryka
Optimize the SqlGuid struct
e07c937 
Optimizes the SqlGuid struct according to the idea
from dotnet#51836.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 20, 2022
@ghost
Copy link

ghost commented Jul 20, 2022

Tagging subscribers to this area: @cheenamalhotra, @David-Engel
See info in area-owners.md if you want to be subscribed.

Issue Details

Optimizes the SqlGuid struct according to the idea
from #51836.

Author: MichalPetryka
Assignees: -
Labels:

area-System.Data.SqlClient
Milestone: -

@MichalPetryka MichalPetryka mentioned this pull request Jul 20, 2022
Closed
GrabYourPitchforks
GrabYourPitchforks reviewed Jul 20, 2022
View reviewed changes
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated Show resolved Hide resolved
src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs Outdated
void ISerializable.GetObjectData(SerializationInfo si, StreamingContext sc)
{
ArgumentNullException.ThrowIfNull(si);
si.AddValue("m_value", ToByteArray());
Copy link
Member

Choose a reason for hiding this comment

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

If the behavior of ToByteArray is reverted, this will have to change.

@Wraith2 Wraith2 mentioned this pull request Jul 23, 2022
Merged
@ghost ghost closed this Sep 11, 2022
@ghost
Copy link

ghost commented Sep 11, 2022

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@MichalPetryka
Copy link
Contributor Author

@David-Engel The bot closed this, could it be reopened?

@stephentoub stephentoub reopened this Sep 11, 2022
@ghost
Copy link

ghost commented Oct 12, 2022

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost closed this Oct 12, 2022
@teo-tsirpanis teo-tsirpanis reopened this Oct 18, 2022
@ghost ghost closed this Nov 17, 2022
@ghost
Copy link

ghost commented Nov 17, 2022

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@teo-tsirpanis teo-tsirpanis reopened this Nov 17, 2022
@MichalPetryka MichalPetryka marked this pull request as ready for review November 17, 2022 23:10
@MichalPetryka
Copy link
Contributor Author

@David-Engel @Wraith2 So what's the status on this?

@build-analysis build-analysis bot mentioned this pull request Nov 18, 2022
Open
David-Engel
David-Engel approved these changes Dec 13, 2022
View reviewed changes
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.

Sorry, this fell off my radar in draft so long and the last mention got buried under other emails. I'm good with the changes.

@David-Engel
Copy link
Contributor

If @GrabYourPitchforks is good with the changes, they can be merged.

@danmoseley
Copy link
Member

@jeffhandley is there anyone that can help look at this one

@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 24, 2023
@build-analysis build-analysis bot mentioned this pull request Jan 29, 2023
Closed
@jeffhandley
Copy link
Member

The failing CI leg is unrelated:

@jeffhandley jeffhandley merged commit bf5afd4 into dotnet:main Jan 30, 2023
@jeffhandley
Copy link
Member

Thank you for this contribution, @MichalPetryka! This change will be included in .NET 8 Preview 1.

@jeffhandley jeffhandley added this to the 8.0.0 milestone Jan 30, 2023
@Wraith2 Wraith2 mentioned this pull request Feb 22, 2023
Closed
1 task
@ghost ghost locked as resolved and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Wraith2 Wraith2 Wraith2 left review comments

@Rob-Hague Rob-Hague Rob-Hague left review comments

@GrabYourPitchforks GrabYourPitchforks GrabYourPitchforks left review comments

@madelson madelson madelson left review comments

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

@jeffhandley jeffhandley jeffhandley approved these changes

Assignees
No one assigned
Labels
area-System.Data.SqlClient community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
10 participants
@MichalPetryka @David-Engel @danmoseley @jeffhandley @madelson @GrabYourPitchforks @Rob-Hague @Wraith2 @stephentoub @teo-tsirpanis