Closed deeprobin closed 2 years ago
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.
I am not sure if a certain comment can be removed because the corresponding issue was closed.
The issue links are typically used to provide additional context, typically about the corner case that the code is dealing with. It is fine for the linked issue to be closed.
Are you in essence trying to propose an amendment to the coding style with a rule that says "Comments should not contain links to closed issues"? I am not sure whether rule like would an improvement.
Tagging subscribers to this area: @dotnet/area-meta See info in area-owners.md if you want to be subscribed.
Author: | deeprobin |
---|---|
Assignees: | - |
Labels: | `area-Meta`, `untriaged` |
Milestone: | - |
I think it would be useful if you could update the tool to also show an excerpt from the source file.
Code with comments like "we have to do this workaround until #1234567 is fixed
" where 1234567 is closed might need attention. Comments like "this fixes the issue described in #1234567" are fine and provide useful context.
No I am not trying to propose that you remove all issue comments in general. My main point was that there are many comments that rather describe a workaround of an issue. In the meantime, the issue may have been closed.
I think it would be useful if you could update the tool to also show an excerpt from the source file.
Code with comments like "
we have to do this workaround until #1234567 is fixed
" where 1234567 is closed might need attention. Comments like "this fixes the issue described in #1234567" are fine and provide useful context.
I have adjusted that accordingly. The current results are now in the top post.
Now I filter by specific keywords using contains:
[ActiveIssue(
fix
resolve
workaround
work around
for now
temporarily
currently
I have pushed my current source code to a GitHub repo.
Note that this is not pretty code and I wrote the program very fast and sloppy.
If I have enough time I can refactor the tool accordingly.
You can remove the several that refer to .NET Framework -- we treat the .NET Framework behaviors as "fixed" so if we're working around something in it, we will forever. The issue number explains why.
This issue has been marked needs-author-action
since it may be missing important information. Please refer to our contribution guidelines for tips on how to report issues effectively.
I took a look at a few, some are interesting, some are intentional. IMO this needs some human supervision on the tool. I've added check-boxes to your report. I suggest that folks collaborate on assessing them. For each one consider the following:
@ericstj Super. Thank you very much.
Unfortunately, I can't comment on or reactivate the closed issues, as most of them are marked as "locked".
I can't comment on or reactivate the closed issues, as most of them are marked as "locked".
@deeprobin I was about to start unlocking them, but I think it would be better to just file new issues. That will ensure the issues are routed to the current area owners.
If you would like to open the new issues for each of the remaining instances, please go ahead and do so. Or let us know if you need us to open the issues on your behalf. Thanks for taking this initiative!
This issue has been marked needs-author-action
since it may be missing important information. Please refer to our contribution guidelines for tips on how to report issues effectively.
@jeffhandley
I can't comment on or reactivate the closed issues, as most of them are marked as "locked".
@deeprobin I was about to start unlocking them, but I think it would be better to just file new issues. That will ensure the issues are routed to the current area owners.
I've already opened a few issues and accordingly marked the item at the top of the listing with the appropriate follow up issue and marked it as done in our list.
If you would like to open the new issues for each of the remaining instances, please go ahead and do so. Or let us know if you need us to open the issues on your behalf. Thanks for taking this initiative!
However, this is also not done in 2 minutes, so I don't mind support in opening Issues on my behalf. Might be a useful sideline when waiting for all CI pipelines to pass.
Afterwards, we should consider whether such a process should be carried out every few months, as certain issues are likely to be closed in the meantime.
All of the identified cases have been fielded and the issue description is updated to reflect the status for each instance. Several follow-up issues were filed, a couple issues were reopened, and a few comments are being updated in #65932.
Thanks for generating this report and iterating on it to help make it addressable, @deeprobin. We would welcome having this report produced every few months or so if you'd like to do that. And the single issue formatted as we landed here ended up being effective.
Thanks @jeffhandley, for taking care of this.
I would think it would be useful to set up a GitHub workflow of the schedule type for this, which would run this tool every 3 months, for example. If necessary we could merge this into the runtime repo or alternatively rewrite the tool in C# (which at best also creates the issues automatically).
I'll also look at writing a GitHub Action soon to see if that would be useful.
Before we pursue automating it, I think it would be best to conduct a couple more manual runs to assess the reoccurring value. Some of the issues found this round were years old and it's unclear to me how frequently we're accruing new "debt."
Every now and then issue URLs are written into the code, for example to describe a workaround around a certain bug.
This creates remnants in the code. I have written a small tool, which uses the GitHub API to find all URLs and checks whether this is a closed issue and then exports the whole thing to me as Markdown [^1].
But I am not sure if a certain comment can be removed because the corresponding issue was closed.
I think we should just look at the following code parts and check that.
Find Closed Issues Tool Results (18.01.2022)
File
src/tests/JIT/Methodical/structs/systemvbringup/structpinvoketests.cs
(File Position 3633-3678)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/tests/JIT/Methodical/structs/systemvbringup/structpinvoketests.cs#L171-L175
File
src/coreclr/pal/src/include/pal/mutex.hpp
(File Position 5384-5430)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/coreclr/pal/src/include/pal/mutex.hpp#L122-L126
File
src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Operators.vb
(File Position 225458-225503)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Operators.vb#L4592-L4596
File
src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs
(File Position 6802-6848)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs#L154-L158
File
src/libraries/System.Private.CoreLib/src/System/Span.cs
(File Position 7264-7310)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Private.CoreLib/src/System/Span.cs#L159-L163
File
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs
(File Position 59996-60042)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs#L1329-L1333
File
src/libraries/System.ComponentModel.Annotations/tests/System/ComponentModel/DataAnnotations/DataTypeAttributeTests.cs
(File Position 3236-3283)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.ComponentModel.Annotations/tests/System/ComponentModel/DataAnnotations/DataTypeAttributeTests.cs#L72-L76
File
src/mono/mono/utils/mono-threads-windows.c
(File Position 4766-4813)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/mono/mono/utils/mono-threads-windows.c#L111-L115
File
src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/IsolatedStorageFile.cs
(File Position 1526-1572)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/IsolatedStorageFile.cs#L38-L42
File
src/libraries/Common/src/Extensions/ParameterDefaultValue/ParameterDefaultValue.netstandard.cs
(File Position 782-828)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/Common/src/Extensions/ParameterDefaultValue/ParameterDefaultValue.netstandard.cs#L22-L26
File
src/libraries/System.Runtime/tests/System/TupleTests.cs
(File Position 11829-11875)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Runtime/tests/System/TupleTests.cs#L218-L222
File
src/libraries/System.ValueTuple/tests/ValueTupleTests.cs
(File Position 12672-12718)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.ValueTuple/tests/ValueTupleTests.cs#L223-L227
File
src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs
(File Position 18733-18780)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs#L401-L405
File
src/tests/JIT/SIMD/Vector3GetHash.cs
(File Position 379-425)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/tests/JIT/SIMD/Vector3GetHash.cs#L5-L9
File
src/libraries/System.Drawing.Common/tests/Imaging/ImageFormatTests.cs
(File Position 5736-5782)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Drawing.Common/tests/Imaging/ImageFormatTests.cs#L94-L98
File
src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs
(File Position 6751-6797)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs#L145-L149
File
src/libraries/System.IO.FileSystem/tests/Directory/EnumerableTests.cs
(File Position 586-633)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.IO.FileSystem/tests/Directory/EnumerableTests.cs#L14-L18
File
src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs
(File Position 42960-43006)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs#L391-L395
File
src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/SettingElementTests.cs
(File Position 1871-1917)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/SettingElementTests.cs#L53-L57
File
src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs
(File Position 34725-34771)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs#L832-L836
File
src/libraries/System.Private.Xml/tests/XmlReader/Tests/ReaderEncodingTests.cs
(File Position 342-388)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Private.Xml/tests/XmlReader/Tests/ReaderEncodingTests.cs#L8-L12
File
src/libraries/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs
(File Position 5028-5074)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs#L118-L122
File
eng/codeOptimization.targets
(File Position 723-769)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/eng/codeOptimization.targets#L8-L12
File
src/coreclr/jit/CMakeLists.txt
(File Position 388-434)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/coreclr/jit/CMakeLists.txt#L7-L11
File
src/libraries/shims/generated/Directory.Build.props
(File Position 1110-1156)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/shims/generated/Directory.Build.props#L23-L27
File
src/tests/JIT/opt/OSR/tailrecursetry.csproj
(File Position 260-306)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/tests/JIT/opt/OSR/tailrecursetry.csproj#L5-L9
File
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/HalfHelpers.cs
(File Position 374-420)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/HalfHelpers.cs#L9-L13
File
src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs
(File Position 3321-3367)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs#L63-L67
File
src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs
(File Position 1362-1408)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs#L33-L37
File
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.cs
(File Position 2889-2935)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.cs#L40-L44
File
src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs
(File Position 22006-22053)https://github.com/dotnet/runtime/blob/f04a24249835096eea1a1a66e4af03cfec5ed32b/src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs#L511-L515
[^1]: Tool source code