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

Cleanup IOleWindow #1868

Merged
merged 1 commit into from
Sep 12, 2019
Merged

Cleanup IOleWindow #1868

merged 1 commit into from
Sep 12, 2019

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Sep 11, 2019 •
edited
Loading

Proposed Changes

  • Extract interface to Interop
  • Make it more resilient
  • Cleanup cases of ContextSensitiveHelp/GetWindow: use PreserveSig, return HRESULT, harden against invalid pointers passed from native code, use BOOL instead of int
Microsoft Reviewers: Open in CodeFlow

@hughbe hughbe requested a review from a team as a code owner September 11, 2019 17:24
@weltkante weltkante mentioned this pull request Sep 12, 2019
Closed
@codecov
Copy link

codecov bot commented Sep 12, 2019 •
edited
Loading

Codecov Report

Merging #1868 into master will increase coverage by 0.0192%.
The diff coverage is 7.31707%.

@@                 Coverage Diff                 @@
##              master       #1868         +/-   ##
===================================================
+ Coverage   26.53953%   26.55873%   +0.01919%     
===================================================
  Files            846         846                 
  Lines         267062      267012         -50     
  Branches       37880       37883          +3     
===================================================
+ Hits           70877       70915         +38     
+ Misses        191163      191058        -105     
- Partials        5022        5039         +17
Flag Coverage Δ
#Debug 26.55873% <7.31707%> (+0.0192%) ⬆️
#production 26.55873% <7.31707%> (+0.0192%) ⬆️
#test 100% <ø> (ø) ⬆️

RussKie
RussKie requested changes Sep 12, 2019
View reviewed changes
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Please break the PR into two distinct PRs - one cleaning IOleWindow, and another cleaning NativeMethods.

src/Common/src/UnsafeNativeMethods.cs Outdated Show resolved Hide resolved
@ghost ghost added the 📭 waiting-author-feedback The team requires more information from the author label Sep 12, 2019
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Sep 12, 2019
@RussKie RussKie added this to the 5.0 milestone Sep 12, 2019
@RussKie RussKie added code cleanup cleanup code for unused apis/properties/comments - no functional changes. enhancement Product code improvement that does NOT require public API changes/additions labels Sep 12, 2019
@RussKie
Copy link
Member

RussKie commented Sep 12, 2019

  • Cleanup cases of ContextSensitiveHelp/GetWindow

Could you please clarify this

@hughbe
Copy link
Contributor Author

hughbe commented Sep 12, 2019

Could you please clarify this

Done

@hughbe hughbe mentioned this pull request Sep 12, 2019
Merged
@RussKie RussKie merged commit d22d6ee into dotnet:master Sep 12, 2019
@hughbe hughbe deleted the IOleWindow branch September 12, 2019 10:00
@RussKie RussKie mentioned this pull request Mar 4, 2021
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@weltkante weltkante weltkante left review comments

@RussKie RussKie RussKie approved these changes

Assignees
No one assigned
Labels
code cleanup cleanup code for unused apis/properties/comments - no functional changes. enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Milestone
5.0 Previews 1-4
Development

Successfully merging this pull request may close these issues.
3 participants
@hughbe @RussKie @weltkante