Fix editorConfig for VB, Suppress IDE001 and IDE002, clean up VB Code and XML Document formatting issues by paul1956 · Pull Request #11653 · dotnet/winforms

paul1956 · 2024-07-11T04:46:22Z

First Fix for #11215 VB Code Cleanup

Proposed changes

Suppress IDE001 and IDE002 in VB that will be needed to address Future PR feedback and make code easier to read
Remove ByVal in VB Templates

Customer Impact

They will not need to remove ByVal when creating new Forms from Templates

Regression?

No

Risk

None

Test methodology

No code changes ByVal is VB Default and is not needed in source

This simple PR will be followed by individual tests, that will test all changes in Future VB Changes

Microsoft Reviewers: Open in CodeFlow

# IDE0001: severity = silent # IDE0002: severity = silent

codecov · 2024-07-11T05:23:01Z

Codecov Report

Attention: Patch coverage is 49.10314% with 227 lines in your changes missing coverage. Please review.

Project coverage is 74.71460%. Comparing base (88d7659) to head (0546d86).
Report is 15 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #11653         +/-   ##
===================================================
+ Coverage   74.57429%   74.71460%   +0.14030%     
===================================================
  Files           3041        3043          +2     
  Lines         629823      630427        +604     
  Branches       46846       46847          +1     
===================================================
+ Hits          469686      471021       +1335     
+ Misses        156783      156041        -742     
- Partials        3354        3365         +11

Flag	Coverage Δ
Debug	`74.71460% <49.10314%> (+0.14030%)`	⬆️
integration	`17.95627% <13.78446%> (+0.07496%)`	⬆️
production	`47.72380% <43.35840%> (+0.24866%)`	⬆️
test	`96.98592% <97.87234%> (+0.00756%)`	⬆️
unit	`44.76852% <29.57393%> (+0.20347%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Ignore ExceptionUtils and Utils they are being completely replaced and VbUtils is temporary it will be replaced by ExUtils

lonitra · 2024-07-15T17:28:14Z

+        'Lazy-initialized and cached log object.
+        Private _log As Logging.Log
+        ' The executing application (the EntryAssembly)
+        Private _info As AssemblyInfo


Could these be moved to the beginning of the class?

@lonitra I had planned a separate PR to move all privates to beginning of files (right now depending on the file they are all over the place and not even together) and sort them, but give the desire to have no code changes I thorough it would not be accepted in this PR. I use CodeMaid to do the sorting, some comment changes in this PR and to prepare for the Sort.

Please advise on how you want to handle the sorting, here or in a follow-on PR?

Handling in a follow up PR sounds reasonable

@lonitra thanks I think it is too big a change. I have done it in PR #9867 and it changes almost every line using automatic tool. All the new code is already properly sorted.

lonitra · 2024-07-15T17:44:49Z

                Else
                    'The name is not in the format we're expecting so return an empty string
-                    Title = ""
+                    title = ""


Suggested change

title = ""

title = string.Empty

@lonitra I go back and forth on this, since .Net 2.0 they generate identical code in VB. I can do whatever you want.
Please advise and I will make them all the same?

In c# we typically prefer string.Empty over "" imho we should prefer string.Empty here as well.

OK I will do them all. No need to comment on each one, updating now.

lonitra · 2024-07-15T17:49:23Z

+        ' Taken from appConfig
+        Private Const DEFAULT_FILE_LOG_TRACE_LISTENER_NAME As String = "FileLog"


Let's move these and the above to the beginning of the class as well

@lonitra based on this comment and others I will sort all the files using CodeMaid.

…icationServices/CantStartSingleInstanceException.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

…icationServices/ConsoleApplicationBase.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

…icationServices/User.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

…ers/NativeTypes.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

…icationServices/ConsoleApplicationBase.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

paul1956 · 2024-07-15T21:44:09Z

@lonitra by changing "" to String.Empty a lot of additional files have changed that I have already cleaned up in a follow-on PR

Do you want me to clean up those, now?

lonitra · 2024-07-15T22:57:29Z

@lonitra by changing "" to String.Empty a lot of additional files have changed that I have already cleaned up in a follow-on PR

Do you want me to clean up those, now?

Is the follow up PR code clean up only? If not, let's go ahead and take care of that in this PR to reduce overhead in the other.

…c/Devices/NetworkTests.cs Co-authored-by: Loni Tra <lonitra@microsoft.com>

lonitra

LGTM 🥳 Please update the PR title to better reflect the changes here

paul1956 · 2024-07-16T20:59:42Z

@lonitra Do I update title on GitHub or with Visual Studio, still confused about process.

lonitra · 2024-07-16T21:15:52Z

Just on github. There should be an 'Edit' button next to the title of your PR at the top which will allow you to make changes to the PR title.

…emove in future PR

KlausLoeffelmann

This looks good to me in general, and I would not have any issues to take this.
I only looked here and there, but couldn't do a full review.
Doing this literately seems to be working this way, I actually like this!

@lonitra - if you feel OK with this, I am OK merging this!

KlausLoeffelmann · 2024-07-19T02:50:54Z

+        ''' <param name="connectionTimeout">Time allotted before giving up on a connection</param>
        ''' <param name="onUserCancel">Indicates what to do if user cancels dialog (either throw or do nothing)</param>
        Public Sub UploadFile(sourceFileName As String,
                              address As Uri,


Do we want to keep this kind of formatting or align it with C#?
(We can do that in a separate PR.)

We should align with C#, but let's do that in a separate PR as this is getting large.

Could someone explain how formatting differs? C# formatting, I thought is largely controlled by VS and .editorConfig, VB is manual but what are the differences that need to be addressed?

When signatures are long and need to be broken up, our C# style guidelines is to break each argument of the signature on its own line with one indent in. Unfortunately we haven't gotten to specifying vb style guidelines as a team, but for the most part we'd want to align with C#. So, for this particular method we'd want to it to look like:

Public Sub UploadFile( sourceFileName As String, address As Uri, networkCredentials As ICredentials, showUI As Boolean, connectionTimeout As Integer, onUserCancel As UICancelOption)

paul1956 · 2024-07-19T05:01:08Z

@KlausLoeffelmann I have a full set of tests that has close to 100% coverage that will be added after this is merged. All tests ran before the changes and also pass after changes.

lonitra

Thank you! 🥳

Fix editorConfig for VB

d3dcc5d

# IDE0001: severity = silent # IDE0002: severity = silent

paul1956 requested a review from a team as a code owner July 11, 2024 04:46

dotnet-policy-service Bot assigned paul1956 Jul 11, 2024

paul1956 mentioned this pull request Jul 11, 2024

Code cleanup only and Add VB Tests #11215

Closed

elachlan added the waiting-review This item is waiting on review by one or more members of team label Jul 11, 2024

lonitra reviewed Jul 11, 2024

View reviewed changes

Comment thread .editorconfig

paul1956 added 2 commits July 12, 2024 15:39

Split files and cleanup

a8f2b18

Clean up comments and fix errors for Option OptionExplicit On

2de2f62

Ignore ExceptionUtils and Utils they are being completely replaced and VbUtils is temporary it will be replaced by ExUtils

lonitra reviewed Jul 15, 2024

View reviewed changes

lonitra added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Jul 15, 2024

dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Jul 15, 2024

paul1956 and others added 6 commits July 15, 2024 14:07

Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/Appl…

996dd20

…icationServices/CantStartSingleInstanceException.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/Appl…

a3ed916

…icationServices/ConsoleApplicationBase.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/Appl…

c742aa1

…icationServices/User.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/Help…

0be816b

…ers/NativeTypes.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/Appl…

553036c

…icationServices/ConsoleApplicationBase.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>

Replace douple quote ("") where String.Empty

1603a6b

Sync PR Feedback

ebb603f

lonitra reviewed Jul 15, 2024

View reviewed changes

Comment thread src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasic/Devices/NetworkTests.cs Outdated

Comment thread src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasic/Devices/NetworkTests.cs Outdated

lonitra added the waiting-author-feedback The team requires more information from the author label Jul 15, 2024

Fix comments and PR feedback

4e05dc9

dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Jul 15, 2024

paul1956 added 4 commits July 15, 2024 16:33

Replace "" with String.Empty

b95b29a

Fix XML Comments

6d14206

Add See Cref's

66d3d08

Minor Cleanup of ExceptionUtils

be780d8

paul1956 and others added 4 commits July 15, 2024 17:52

Update src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasi…

8ce7442

…c/Devices/NetworkTests.cs Co-authored-by: Loni Tra <lonitra@microsoft.com>

Update src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasi…

e453e73

…c/Devices/NetworkTests.cs Co-authored-by: Loni Tra <lonitra@microsoft.com>

Fix extra paren from PR Review

41ff2be

Remove comment download is test now

ad33dba

lonitra previously approved these changes Jul 16, 2024

View reviewed changes

paul1956 changed the title ~~Fix editorConfig for VB and Supress IDE001 and IDE002~~ Fix editorConfig for VB, Suppress IDE001 and IDE002, clean up VB Code and XML Document formatting issues Jul 16, 2024

paul1956 added 2 commits July 16, 2024 16:42

Merge branch 'master' into Fix-editorConfig-Templates-for-VB-Review-1st

2c2bebc

Remove Option statement from VB because its in Project file

ddf75b5

paul1956 dismissed lonitra’s stale review via ddf75b5 July 16, 2024 23:52

Rename Utils to Utils1 to avoid temporary name clash. Class will be r…

22c0b9e

…emove in future PR

lonitra reviewed Jul 17, 2024

View reviewed changes

Comment thread src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/CompilerServices/Utils1.vb Outdated

lonitra added the waiting-author-feedback The team requires more information from the author label Jul 17, 2024

dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Jul 18, 2024

lonitra added the waiting-author-feedback The team requires more information from the author label Jul 18, 2024

Revert rename of Utils back to Util

0546d86

dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Jul 18, 2024

KlausLoeffelmann reviewed Jul 19, 2024

View reviewed changes

lonitra approved these changes Jul 19, 2024

View reviewed changes

lonitra merged commit 9278903 into dotnet:main Jul 19, 2024

dotnet-policy-service Bot added this to the 9.0 Preview7 milestone Jul 19, 2024

paul1956 deleted the Fix-editorConfig-Templates-for-VB-Review-1st branch July 19, 2024 21:18

github-actions Bot locked and limited conversation to collaborators Aug 19, 2024

		' Taken from appConfig
		Private Const DEFAULT_FILE_LOG_TRACE_LISTENER_NAME As String = "FileLog"

Uh oh!

Conversation

paul1956 commented Jul 11, 2024 • edited by dotnet-policy-service Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Test methodology

Microsoft Reviewers: Open in CodeFlow

Uh oh!

codecov Bot commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paul1956 commented Jul 15, 2024

Uh oh!

lonitra commented Jul 15, 2024

Uh oh!

Uh oh!

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

paul1956 commented Jul 16, 2024

Uh oh!

lonitra commented Jul 16, 2024

Uh oh!

Uh oh!

KlausLoeffelmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul1956 commented Jul 19, 2024

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

paul1956 commented Jul 11, 2024 •

edited by dotnet-policy-service Bot

Loading

codecov Bot commented Jul 11, 2024 •

edited

Loading