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

Merging common code bases AdapterUtil #1306

Merged
merged 8 commits into from
Oct 12, 2021
Merged

Merging common code bases AdapterUtil #1306

merged 8 commits into from
Oct 12, 2021

Conversation

DavoudEshtehari
Copy link
Contributor

@DavoudEshtehari DavoudEshtehari commented Oct 1, 2021 •
edited
Loading

  • Related issue Possible mergeable classes for merging code bases #1261.
  • Common type: ADP
    • All the common code-bases are pruned and moved into a new file with the best existing implementation with new styles.
    • Each project has its own specific implementation in a separate file.
    • By a minor change in StringsHelper I managed to replace Format() with GetString() for current functions in NetCore, and avoided splitting code.
    • Null reference functions have been removed.

@DavoudEshtehari DavoudEshtehari added the ➕ Code Health Issues/PRs that are targeted to source code quality improvements. label Oct 1, 2021
@DavoudEshtehari DavoudEshtehari added this to the 4.0.0-preview3 milestone Oct 1, 2021
@DavoudEshtehari DavoudEshtehari mentioned this pull request Oct 1, 2021
Open
cheenamalhotra
cheenamalhotra approved these changes Oct 4, 2021
View reviewed changes
Copy link
Member

@cheenamalhotra cheenamalhotra left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.

src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtilCommon.cs Outdated Show resolved Hide resolved
@DavoudEshtehari
remove more null references
e5eecef 
Unify the files in common folder
@DavoudEshtehari
Merge remote-tracking branch 'UpStream/main' into Adhoc/MergeAdapterUtil
597a935 
# Conflicts:
#	src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommandSet.cs
@DavoudEshtehari
remove s_strEmpty
ff839c3
@cheenamalhotra cheenamalhotra merged commit 31f566a into dotnet:main Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wraith2 Wraith2 Wraith2 left review comments

@lcheunglci lcheunglci lcheunglci approved these changes

@johnnypham johnnypham johnnypham approved these changes

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

Assignees
No one assigned
Labels
➕ Code Health Issues/PRs that are targeted to source code quality improvements.
Projects
None yet
Milestone
4.0.0-preview3
Development

Successfully merging this pull request may close these issues.
5 participants
@DavoudEshtehari @Wraith2 @cheenamalhotra @johnnypham @lcheunglci