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 to shared - TdsParserStaticMethods #1603

Closed

Conversation

lcheunglci
Copy link
Contributor

Relates to #1261. Merged netfx into netcore for TdsParserStaticMethods, and moving it to shared src and updating file to conform with coding style.

// For ProjectK\CoreCLR we don't want to take a dependency on the registry to try to read a value
// that isn't usually set, so we'll just use a random value each time instead
if (null == s_nicAddress)
{
byte[] newNicAddress = new byte[TdsEnums.MAX_NIC_SIZE];
Random random = new Random();
Random random = new();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should dispose the Random instance, there was a PR recently to make sure that was done elsewhere in this library.
The Type name = new() is less clear than var name = new Type() imo and I thought we had added an editorconfig entry to prevent auto suggestion of the first version in the main library projects.

@Wraith2
Copy link
Contributor

Wraith2 commented May 4, 2022

I'm not a big fan of using #ifdef's in the main files. I've not checked if there's a better way to do this but if there is it'd be better to have .netfx.cs and .netcore.cs files with parts of the partial class in them.

@DavoudEshtehari
Copy link
Contributor

Closing this PR since #1588 addresses the same merging.

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