Flow.Launcher icon indicating copy to clipboard operation
Flow.Launcher copied to clipboard

Published 20 hours ago • verified publisher icon Flow-Launcher

Fix (some) build warnings

Open nachmore opened this issue 3 years ago • 7 comments

Primarily fixing a bunch of:

  1. CS1572
  2. CS1573
  3. CS0168
  4. CS8073
  5. CA2200
  6. VSTHRD110
  7. VSTHRD200
  8. VSTHRD105
  9. SYSLIB0013
  10. CS8524

Not a comprehensive fix, but removes a large number of warnings.

nachmore avatar Aug 08 '22 04:08 nachmore

Before: 400 warnings After: 161 warnings

nachmore avatar Aug 08 '22 04:08 nachmore

@nachmore One of the build test has failed. Would you please take a look on what's causing that?

taooceros avatar Aug 08 '22 23:08 taooceros

I'm looking at this now - it's due to a change in behaviour handling null in json. So much for "just replace X with Y" 😜

nachmore avatar Aug 08 '22 23:08 nachmore

Looking at build mismatch...

nachmore avatar Aug 09 '22 00:08 nachmore

I'm going to pause the warnings elimination work as this PR is large enough already. So when folks have time, can you please review these changes, I'll address comments and then continue in a separate PR. Thanks!

nachmore avatar Aug 09 '22 00:08 nachmore

Down to 118.

Could I please get a review of these changes? I created follow up tasks for some of the fundamental comments that were uncovered during this PR (but not directly related to these changes).

nachmore avatar Aug 10 '22 03:08 nachmore

Down to 118.

Could I please get a review of these changes? I created follow up tasks for some of the fundamental comments that were uncovered during this PR (but not directly related to these changes).

Sure, just give me sometime.

taooceros avatar Aug 10 '22 03:08 taooceros

Sure, just give me sometime.

Sorry if it sounded like I was rushing you - I am blown away by the speed of CRs in this project, it's incredible! Thank you for the super quick support...

nachmore avatar Aug 10 '22 18:08 nachmore

Sure, just give me sometime.

Sorry if it sounded like I was rushing you - I am blown away by the speed of CRs in this project, it's incredible! Thank you for the super quick support...

Na you are good. It's always easier for me to review than you writing the code, and it will be super nice to hear feedback quickly when contributing. Let's finish the remaining trivial stuff and merge this one.

taooceros avatar Aug 10 '22 18:08 taooceros

Thank you so much for making such a huge change. There's a small pieces that worth taking a look before merging. After addressing them, I think we shall be able to merge.

Let's wrap this with Task.Run and make the method Async

https://github.com/Flow-Launcher/Flow.Launcher/blob/30669e8c257ce02f08cf763dcd1efaf99950ed81/Plugins/Flow.Launcher.Plugin.Program/Programs/UWP.cs#L421

I think this may not be useful and we are ready to merge.

taooceros avatar Aug 11 '22 14:08 taooceros