Skip to content

Conversation

@softwarepronto
Copy link
Contributor

Summary

The WebClient variable has a using
Using client As New WebClient()

These were missing the using:
Ref data As Stream = client.OpenRead(args(0))
Ref reader As New StreamReader(data)

The order of Close for reader.Close and data.Close was reversed.

Fixes #Issue_Number (if available)

The WebClient variable has a using
Using client As New WebClient()

These were missing the using:
   Ref data As Stream = client.OpenRead(args(0))
   Ref reader As New StreamReader(data)

The order of Close for reader.Close and data.Close was reversed.
@softwarepronto softwarepronto requested a review from a team as a code owner October 22, 2022 12:16
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 22, 2022
@ghost
Copy link

ghost commented Oct 22, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@opbld33
Copy link

opbld33 commented Oct 22, 2022

Learn Build status updates of commit 2f5d4b2:

✅ Validation status: passed

File Status Preview URL Details
snippets/visualbasic/VS_Snippets_Remoting/NCLWebClientUserAgent/VB/useragent.vb ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

End Using need for VB.net
@opbld31
Copy link

opbld31 commented Oct 22, 2022

Learn Build status updates of commit 623f146:

✅ Validation status: passed

File Status Preview URL Details
snippets/visualbasic/VS_Snippets_Remoting/NCLWebClientUserAgent/VB/useragent.vb ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks @softwarepronto

I like this as well.

Let's make this change consistent with the corresponding C# sample PR. Then, it's ready to :shipit:

@softwarepronto softwarepronto changed the title Missing use for Stream and StreamReader variables Missing using for Stream and StreamReader variables Oct 24, 2022
Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

After adding the two suggestions, this LGTM.

@BillWagner BillWagner enabled auto-merge (squash) October 25, 2022 13:20
@opbld31
Copy link

opbld31 commented Oct 25, 2022

Learn Build status updates of commit 695087d:

✅ Validation status: passed

File Status Preview URL Details
snippets/visualbasic/VS_Snippets_Remoting/NCLWebClientUserAgent/VB/useragent.vb ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@BillWagner BillWagner merged commit 0033e0f into dotnet:main Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants