Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Remove unused locals in System.Data.OleDb #42485

Merged
6 commits merged into from
Nov 13, 2019
Merged

Remove unused locals in System.Data.OleDb #42485

6 commits merged into from
Nov 13, 2019

Conversation

eriawan
Copy link
Member

@eriawan eriawan commented Nov 8, 2019

Remove unused locals in System.Data.OleDb and keep some logic using discard.

Partially fix issue #39962

cc @stephentoub

@eriawan eriawan changed the title Remove unused locals in System.Data.OleDband Remove unused locals in System.Data.OleDb Nov 8, 2019
@eriawan
Copy link
Member Author

eriawan commented Nov 12, 2019

@stephentoub
Pardon, could you please review this PR? Thanks in advance! :)

eriawan and others added 5 commits November 14, 2019 01:06
@eriawan @stephentoub
Update src/System.Data.OleDb/src/OleDbDataReader.cs
210fc33 
update as suggested (OleDbDataReader)

Co-Authored-By: Stephen Toub <[email protected]>
@eriawan @stephentoub
Update src/System.Data.OleDb/src/OleDbTransaction.cs
885526a 
Done update as suggested

Co-Authored-By: Stephen Toub <[email protected]>
@eriawan @stephentoub
Update src/System.Data.OleDb/src/RowBinding.cs
0830fb7 
Done update as suggested (RowDataBinding)

Co-Authored-By: Stephen Toub <[email protected]>
@eriawan @stephentoub
Update src/System.Data.OleDb/src/RowBinding.cs
6fcd4fb 
Done as suggested

Co-Authored-By: Stephen Toub <[email protected]>
@eriawan @stephentoub
Update src/System.Data.OleDb/src/SafeHandles.cs
900243b 
Done updating as suggested (SafeHandles)

Co-Authored-By: Stephen Toub <[email protected]>
@eriawan
Copy link
Member Author

eriawan commented Nov 13, 2019

@stephentoub

Thanks for the review and feedback! I have updated those files as suggested.

danmoseley
danmoseley approved these changes Nov 13, 2019
View reviewed changes
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

thanks!

@danmoseley danmoseley added the auto-merge Automatically merge PR once CI passes. label Nov 13, 2019
@ghost
Copy link

ghost commented Nov 13, 2019

Hello @danmosemsft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit eceea8b into dotnet:master Nov 13, 2019
@eriawan
Copy link
Member Author

eriawan commented Nov 15, 2019

Thanks @stephentoub and @danmosemsft !

@eriawan eriawan deleted the remove_unused_variables_System.Data.OleDb branch November 15, 2019 13:18
@danmoseley
Copy link
Member

@eriawan thanks for the contribution -- perhaps when this code has moved to dotnet/runtime (should be open in a week or so) you'd be interested in making another contribution there.

@eriawan
Copy link
Member Author

eriawan commented Nov 15, 2019

@danmosemsft
Thanks for letting me know the update plan about dotnet/runtime!
Ok, is this dotnet/runtime the planned consolidated place for .NET Core repos?
Is it already up in GitHub?

@danmoseley
Copy link
Member

@eriawan yep, please see https://github.com/dotnet/coreclr/issues/27549

@danmoseley
Copy link
Member

Meantime, no reason to wait: you can look for "up for grabs" issues in this repo. We just can't merge anything for a few days.

@karelz karelz added this to the 5.0 milestone Dec 19, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub left review comments

@danmoseley danmoseley danmoseley approved these changes

@cheenamalhotra cheenamalhotra Awaiting requested review from cheenamalhotra

@saurabh500 saurabh500 Awaiting requested review from saurabh500

Assignees
No one assigned
Labels
area-System.Data auto-merge Automatically merge PR once CI passes.
Projects
None yet
Milestone
5.0
Development

Successfully merging this pull request may close these issues.
6 participants
@eriawan @danmoseley @stephentoub @karelz @cheenamalhotra @Dotnet-GitSync-Bot