Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 11, 2025

Comments referencing specific line numbers become misleading when code changes. Found and fixed one instance in the repository.

Change

  • src/Cli/dotnet/Commands/Test/MTP/IPC/NamedPipeServer.cs:239: Replaced "line 103" reference with descriptive text
// Before
// To close gracefully we need to ensure that the client closed the stream line 103.

// After  
// To close gracefully we need to ensure that the client closed the stream and the InternalLoopAsync method has exited.

The comment now describes what it's waiting for (the async loop method to exit after detecting client disconnect) rather than referencing a line number that could shift as code evolves.

Original prompt

This section details on the original issue you should resolve

<issue_title>Any line related comments that aged like milk?</issue_title>
<issue_description>Find all instances of referring to other code by line number or other non descriptive descriptor in this repo, and fix it.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix comments referring to code by line numbers Replace line number reference with descriptive comment Nov 11, 2025
Copilot AI requested a review from nohwnd November 11, 2025 13:38
Copilot finished work on behalf of nohwnd November 11, 2025 13:38
@nohwnd nohwnd marked this pull request as ready for review November 11, 2025 15:30
Copilot AI review requested due to automatic review settings November 11, 2025 15:30
Copilot finished reviewing on behalf of nohwnd November 11, 2025 15:32
@KalleOlaviNiemitalo
Copy link
Contributor

git grep -P -e '[Ll]ine \d' origin/main doesn't find anything else that looks like it should be changed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issue #51645 by replacing a line number reference in a comment with descriptive text, improving code maintainability. Line number references in comments become outdated and misleading as code evolves, making them problematic for future maintenance.

Key Changes:

  • Updated comment in NamedPipeServer.cs to describe what the code is waiting for (InternalLoopAsync method exit after client disconnect) instead of referencing "line 103"

@KalleOlaviNiemitalo
Copy link
Contributor

However, git grep -e '#L' origin/main finds stuff like:

origin/main:documentation/project-docs/Localization.md:This is controlled https://github.com/dotnet/sdk/blob/main/eng/pipelines/templates/jobs/sdk-job-matrix.yml#L86 and requires a change both here and in the loc system to align branches.
origin/main:documentation/project-docs/SDK-PR-guide.md:- [ ] Set [`StabilizePackageVersion`](https://github.com/dotnet/sdk/blob/main/eng/Versions.props#L18) to true
origin/main:documentation/project-docs/developer-guide.md:> If you're building main, we may be using a preview version of the SDK to build as specified in [global.json](../../global.json#L3). We only test preview SDKs with the latest Visual Studio previews so would recommend installing the latest preview build. You can see the preview versions we test with [here](https://learn.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs#preview-versioning)
origin/main:documentation/project-docs/snapshot-based-testing.md:[compare]: ../../test/dotnet.Tests/dotnet.Tests.csproj#L100
origin/main:documentation/project-docs/snapshot-based-testing.md:[update]: ../../test/dotnet.Tests/dotnet.Tests.csproj#L107
origin/main:src/Cli/Microsoft.TemplateEngine.Cli/NewCommandStatus.cs:    ///    https://github.com/dotnet/sdk/blob/main/src/Cli/dotnet/Parser.cs#L148.
origin/main:src/Cli/dotnet/Commands/Test/MTP/Terminal/SystemConsole.cs:        // From https://github.com/dotnet/runtime/blob/main/src/libraries/System.Console/src/System/Console.cs#L236
origin/main:src/Cli/dotnet/SudoEnvironmentDirectoryOverride.cs:        // https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Common/PathUtil/NuGetEnvironment.cs#L139
origin/main:src/Microsoft.CodeAnalysis.NetAnalyzers/docs/NetCore_GettingStarted.md:    - Set `RUNTIMEPACKAGEVERSION` variable with a version value whose major part is equal to the major part of the version the [runtime](https://github.com/dotnet/runtime/blob/main/eng/Versions.props#L53)/[roslyn-analyzers](https://github.com/dotnet/roslyn-analyzers/blob/main/eng/Versions.props#L50) repo is using. Example: `set RUNTIMEPACKAGEVERSION=8.0.0`
origin/main:src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/Compiler/Extensions/IOperationExtensions.cs:        // Copied from roslyn https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs#L25

I omitted the URLs that link to a specific commit ID or tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Any line related comments that aged like milk?

3 participants