Remove ifdefs from winforms by hughbe · Pull Request #3169 · dotnet/winforms

hughbe · 2020-04-30T11:37:27Z

Proposed Changes

Remove never defined ifdefs from winforms
Remove always defined ifdefs from winforms

The remaining list is

DEBUG
~~DEBUG_LAYOUT~~
~~DEBUG_PAINT~~
~~DEBUG_PAINT_ANCHOR~~
~~DEBUG_UPDOWN~~
~~FINALIZATION_WATCH~~
~~GDI_FINALIZATION_WATCH~~
~~LAYOUT_PERFWATCH~~
~~PBRS_PAINT_DEBUG~~
~~TRACK_HDC~~

What should we do with these? I didn't want to delete as they may be useful, but all the above except DEBUG (which should stay, obviously), are never used define. Should we delete this code?

Fixes #39

Microsoft Reviewers: Open in CodeFlow

RussKie

👍

RussKie · 2020-05-01T03:41:12Z

@Tanya-Solyanik any objections?

hughbe · 2020-05-01T09:13:52Z

Also would appreciate some suggestions on the remaining ifdefs listed above. Also, this fixes #39

codecov · 2020-05-07T08:20:42Z

Codecov Report

Merging #3169 into master will decrease coverage by 36.96293%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                  @@
##              master       #3169          +/-   ##
====================================================
- Coverage   98.54350%   61.58057%   -36.96293%     
====================================================
  Files            420        1286         +866     
  Lines         197872      451912      +254040     
  Branches        2756       39504       +36748     
====================================================
+ Hits          194990      278290       +83300     
- Misses          2329      168260      +165931     
- Partials         553        5362        +4809

Flag	Coverage Δ
#Debug	`61.58057% <100.00000%> (-36.96293%)`	⬇️
#production	`32.79798% <100.00000%> (?)`
#test	`98.53340% <ø> (-0.01010%)`	⬇️

JeremyKuhne · 2020-05-22T00:53:57Z

Keep #ifdef DEBUG. We should create an issue to consider how we want to track finalization so that we can shake out issues in disposal as we're clearly having problems in our test runs (to replace the two FINALIZATION_WATCH defines).

codecov · 2020-05-22T09:08:03Z

Codecov Report

Merging #3169 into master will decrease coverage by 30.86467%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                  @@
##              master       #3169          +/-   ##
====================================================
- Coverage   64.68383%   33.81916%   -30.86468%     
====================================================
  Files           1318         888         -430     
  Lines         484206      253723      -230483     
  Branches       39910       36766        -3144     
====================================================
- Hits          313203       85807      -227396     
+ Misses        165627      163178        -2449     
+ Partials        5376        4738         -638

Flag	Coverage Δ
#Debug	`33.81916% <100.00000%> (-30.86468%)`	⬇️
#production	`33.81916% <100.00000%> (+0.00861%)`	⬆️
#test	`?`

RussKie

👍

RussKie · 2020-05-26T04:03:35Z

src/System.Windows.Forms/src/misc/GDI/DeviceContext.cs

 // See the LICENSE file in the project root for more information.

-// #define TRACK_HDC
 // #define GDI_FINALIZATION_WATCH


Looks like this was missed

I think FINALIZATION_WATCH is now back in as requested

Delete remaining except FINALIZATION-WATCH

It is being removed in other places though.
I created #3348 to track/reinstate finalization watch.

Oh, didn't notice it was inconsistent, probably should be updated to either remove all of them or keep all of them

Thats what I did :)

hughbe · 2020-05-26T08:02:42Z

Updated

codecov · 2020-05-26T11:19:36Z

Codecov Report

Merging #3169 into master will increase coverage by 33.94661%.
The diff coverage is n/a.

@@                 Coverage Diff                  @@
##              master       #3169          +/-   ##
====================================================
+ Coverage   64.68383%   98.63044%   +33.94660%     
====================================================
  Files           1318         430         -888     
  Lines         484206      230439      -253767     
  Branches       39910        3142       -36768     
====================================================
- Hits          313203      227283       -85920     
+ Misses        165627        2523      -163104     
+ Partials        5376         633        -4743

Flag	Coverage Δ
#Debug	`98.63044% <ø> (+33.94660%)`	⬆️
#production	`?`
#test	`98.63044% <ø> (-0.05208%)`	⬇️

RussKie · 2020-05-26T11:20:28Z

...and it passed 😕

hughbe requested a review from a team as a code owner April 30, 2020 11:37

ghost assigned hughbe Apr 30, 2020

hughbe force-pushed the ifdefs branch 5 times, most recently from 6766485 to 6db91b7 Compare April 30, 2020 14:50

RussKie previously approved these changes May 1, 2020

View reviewed changes

RussKie added waiting-review This item is waiting on review by one or more members of team code cleanup cleanup code for unused apis/properties/comments - no functional changes. labels May 1, 2020

hughbe dismissed RussKie’s stale review via 278b1d4 May 7, 2020 08:04

hughbe force-pushed the ifdefs branch from 6db91b7 to 278b1d4 Compare May 7, 2020 08:04

RussKie requested a review from JeremyKuhne May 7, 2020 23:49

hughbe force-pushed the ifdefs branch from 278b1d4 to a8cfcce Compare May 20, 2020 08:28

hughbe added 4 commits May 22, 2020 09:41

Remove never defined code

1dd17c7

Remove WGCM_TEST_SUITE andGDI_FONT_CACHE_TRACK

6bcf3ec

Remove always defined OPTIMIZED_MEASUREMENTDC

2ac6c04

Delete remaining except FINALIZATION-WATCH

e12fc0c

hughbe force-pushed the ifdefs branch from a8cfcce to e12fc0c Compare May 22, 2020 08:42

RussKie previously approved these changes May 26, 2020

View reviewed changes

RussKie removed the waiting-review This item is waiting on review by one or more members of team label May 26, 2020

RussKie mentioned this pull request May 26, 2020

Track finalization #3348

Closed

Remove GDI_FINALIZATION_WATCH

00ed426

hughbe dismissed RussKie’s stale review via c7e7a0b May 26, 2020 08:02

Delete FINALIZATION_WATCH

5fe274c

hughbe force-pushed the ifdefs branch from c7e7a0b to 5fe274c Compare May 26, 2020 10:10

This comment has been minimized.

Sign in to view

RussKie merged commit 832abd6 into dotnet:master May 26, 2020

ghost added this to the 5.0 Preview6 milestone May 26, 2020

hughbe deleted the ifdefs branch May 26, 2020 11:49

ghost locked as resolved and limited conversation to collaborators Feb 1, 2022

Conversation

hughbe commented Apr 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Microsoft Reviewers: Open in CodeFlow

Uh oh!

RussKie left a comment

Choose a reason for hiding this comment

Uh oh!

RussKie commented May 1, 2020

Uh oh!

hughbe commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 7, 2020

Codecov Report

Uh oh!

JeremyKuhne commented May 22, 2020

Uh oh!

codecov bot commented May 22, 2020

Codecov Report

Uh oh!

RussKie left a comment

Choose a reason for hiding this comment

Uh oh!

RussKie May 26, 2020

Choose a reason for hiding this comment

Uh oh!

weltkante May 26, 2020

Choose a reason for hiding this comment

Uh oh!

RussKie May 26, 2020

Choose a reason for hiding this comment

Uh oh!

weltkante May 26, 2020

Choose a reason for hiding this comment

Uh oh!

hughbe May 26, 2020

Choose a reason for hiding this comment

Uh oh!

hughbe commented May 26, 2020

Uh oh!

This comment has been minimized.

codecov bot commented May 26, 2020

Codecov Report

Uh oh!

RussKie commented May 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hughbe commented Apr 30, 2020 •

edited

Loading

hughbe commented May 1, 2020 •

edited

Loading