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

Share SqlBuffer #1438

Merged
merged 6 commits into from
Apr 6, 2022
Merged

Share SqlBuffer #1438

merged 6 commits into from
Apr 6, 2022

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Dec 21, 2021

This ports the changes from dotnet/corefx#31044 to netfx. It then shares the resulting SqlBuffer file and uses a SqlBuffer.netfx.cs file to contain the netfx (legacy) methods required for v200 smi support.

@DavoudEshtehari this adds to the codebase merging

@Wraith2
Copy link
Contributor Author

Wraith2 commented Dec 22, 2021

The failures don't appear related to my changes, can someone re-run the failed legs?

@DavoudEshtehari DavoudEshtehari added the ➕ Code Health Changes related to source code improvements label Dec 22, 2021
@DavoudEshtehari DavoudEshtehari added this to the 5.0.0-preview1 milestone Dec 22, 2021
@JRahnama
Copy link
Member

can you address the conflicts here please?

@Wraith2
Copy link
Contributor Author

Wraith2 commented Feb 23, 2022

Done.

_isNull = false;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

instead of doing this could not we have #if directive inside one class? this creates a new file again for netfx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could but it would be inconsistent with the approach I've previously been asked to use. The approach has been to use conditional elements within the project file and avoid #ifdef blocks in individual files favouring totally separate files where possible.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Apr 1, 2022

Rebased and fixed. Please don't make me to this a third time.

@DavoudEshtehari DavoudEshtehari merged commit 187c3e2 into dotnet:main Apr 6, 2022
@Wraith2 Wraith2 deleted the port-datetimespanify branch April 6, 2022 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Changes related to source code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants