winforms
winforms copied to clipboard
Vb code cleanup
Fixes #
Proposed changes
Clean up Visual Basic Code to eliminate suggestions, fixes 85 issues flagged by Visual Studio
Customer Impact
None, if targets developers who get 85 issues that clutter up the Error List
Regression?
No, it more about making code match .editorConfig included with project. Alternativity, .editorConfig could be changed to match the Visual Basic Code but that will cause other cosmetic issues, where other code does not match newer .editorConfig.
Risk
Very low, mostly renaming of local variables, fix spelling errors in comments and adding missing "Public" on public functions, biggest change is If Statement to if Expression.
Test methodology
Code completes, existing tests pass, changes are mostly cosmetic.
This is only in 1 Project written in old Visual Basic
Microsoft Reviewers: Open in CodeFlow
I would be surprised, if the Roslyn team doesn't have a dedicated .editorConfig for VB. Couldn't we steal that from them? I cannot believe that the current setting we're using should be some sort of standard for VB.
@AlekseyTs: Asking for a friend! 😸
Sorry. I prefer to stay oblivious of the .editorConfig stuff. Feel free to check what we have in the Roslyn repo. I personally never reformat entire file, and I would revert spacing changes if VS were to make them without me explicitly looking to make the changes. Regardless of what .editorConfig has.
I would be surprised, if the Roslyn team doesn't have a dedicated .editorConfig for VB. Couldn't we steal that from them? I cannot believe that the current setting we're using should be some sort of standard for VB.
@AlekseyTs: Asking for a friend! 😸
@KlausLoeffelmann Roslyn has 1 editorConfig for the solution (VB and C#), a few ancillary repos have their own which inherit from the root one.
Spacing is one issue where blank lines after classes, punctions is currently specified and saving applies the formatting. it's possible there is a VS Setting to prevent this the auto formating but I haven't found it.
A separate issue is preferring If Expressions for C# and If Statements for VB. This is controlled by .editorConfig and right now they share the same settings to prefer If Expressions.
@KlausLoeffelmann I have manually removed all the extra lines that VS wants to add, the biggest manual change is separating Imports by root type. It that is a setting I don't know how to turn it off.
I have added ExceptionUtils and other prefixes like it back. The code was not consistent in how/when it used ExceptionUtils directly vs in an Imports. I have made all uses follow below.
Imports ExUtils = Microsoft.VisualBasic.CompilerServices.ExceptionUtils
@KlausLoeffelmann Ping This is all cleaned up with updated and separated editorConfig and updated VB project file with options specified. I think I removed all the extra blank lines, but any editing in VS will bring them back. Someone that knows about controlling the blank lines should review and recommend a fix that is more automatic and not requiring removing the blanks using Notepad. This should be reviewed and merged before the Fix to remove WebClient which has all the same formatting issues.
FYI there is no consistency on use of blank lines before or after End Class and after End NameSpace or after File header in original source.
I need to take a bit more time, which I don't have right now. I'll dial back to this in a bit. Sorry for the delay!
I need to take a bit more time, which I don't have right now. I'll dial back to this in a bit. Sorry for the delay!
@KlausLoeffelmann no issue, I continue to fix formatting differences between different files. I will continue to keep this PR up to date with Main.
When you get the chance, can you also start using the name "main" for the equivalence of our main branch to avoid confusion? Thx!
@KlausLoeffelmann gentle ping to keep this on your radar to take this to completion
Yes, Ma'am. As soon as I can get to it! Give me a bit more time - thanks for pinging me!
Ping, is there an ETA when someone might review this?
Codecov Report
Attention: Patch coverage is 81.48496%
with 197 lines
in your changes are missing coverage. Please review.
Project coverage is 73.31195%. Comparing base (
fd95fcd
) to head (24e5ab5
).
Additional details and impacted files
@@ Coverage Diff @@
## main #10117 +/- ##
===================================================
+ Coverage 73.22354% 73.31195% +0.08840%
===================================================
Files 3096 3104 +8
Lines 633620 634254 +634
Branches 47336 47341 +5
===================================================
+ Hits 463959 464984 +1025
+ Misses 166112 165730 -382
+ Partials 3549 3540 -9
Flag | Coverage Δ | |
---|---|---|
Debug | 73.31195% <81.48496%> (+0.08840%) |
:arrow_up: |
integration | 18.41764% <1.45985%> (-0.00230%) |
:arrow_down: |
production | 46.84488% <52.06813%> (+0.13442%) |
:arrow_up: |
test | 94.98804% <100.00000%> (+0.00853%) |
:arrow_up: |
unit | 43.74090% <50.85158%> (+0.12442%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
This is replace by other PR's and is unnecessary