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

Move to Shared for SqlEnums.cs #1317

Merged
merged 38 commits into from
Oct 15, 2021
Merged

Conversation

lcheunglci
Copy link
Contributor

Relates to #1261 . I merged the netfx changes into netcore, and move it to shared src. I updated to the code to conform with the coding style, but left the IDE1006 ones because renaming the static variables with _s and internal read-only fields with _ affected other files i.e. SqlDataReader, SqlDataRecord, SqlUtil, SqlParameter, TdsParser, SqlComand, and more, which are all on the list to be merged, so I wanted to keep the changes small.

@Kaur-Parminder Kaur-Parminder added the ➕ Code Health Issues/PRs that are targeted to source code quality improvements. label Oct 5, 2021
@Kaur-Parminder Kaur-Parminder added this to the 4.0.0-preview3 milestone Oct 5, 2021
Copy link
Contributor

@johnnypham johnnypham left a comment

Choose a reason for hiding this comment

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

There were some mdac comments that weren't brought over. It was confirmed that they should remain.

Other than that, LGTM.

@Kaur-Parminder
Copy link
Contributor

Kaur-Parminder commented Oct 13, 2021

NIT: @lcheunglci All methods from TypeId to GetMetaTypeFromValue can use body expressions except GetMetaTypeFromSqlDbType.

@JRahnama JRahnama merged commit 12fafc2 into dotnet:main Oct 15, 2021
@lcheunglci lcheunglci deleted the MergeShared-SqlEnums branch October 15, 2021 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Issues/PRs that are targeted to source code quality improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants