-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup Issue-URLs in Code #63902
Comments
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. |
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 Issue DetailsEvery 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 (17.01.2022)Issue 882References
Issue 936References
Issue 1027References
Issue 1349References
Issue 1423References
Issue 1780References
Issue 1913References
Issue 4069References
Issue 4680References
Issue 4941References
Issue 5552References
Issue 5778References
Issue 5924References
Issue 6014References
Issue 6775References
Issue 6796References
Issue 6879References
Issue 7103References
Issue 7141References
Issue 7474References
Issue 7919References
Issue 8608References
Issue 8683References
Issue 8730References
Issue 8896References
Issue 8897References
Issue 9014References
Issue 9401References
Issue 9495References
Issue 9565References
Issue 9635References
Issue 9899References
Issue 10607References
Issue 10950References
Issue 11191References
Issue 11636References
Issue 11637References
Issue 11678References
Issue 12392References
Issue 13382References
Issue 13816References
Issue 14472References
Issue 14545References
Issue 14654References
Issue 14793References
Issue 14794References
Issue 14885References
Issue 15152References
Issue 15182References
Issue 15397References
Issue 15650References
Issue 15690References
Issue 16050References
Issue 16236References
Issue 16737References
Issue 17190References
Issue 17618References
Issue 18037References
Issue 18208References
Issue 18254References
Issue 18664References
Issue 18676References
Issue 18844References
Issue 19106References
Issue 19265References
Issue 19275References
Issue 19277References
Issue 19584References
Issue 20080References
Issue 20097References
Issue 20232References
Issue 20332References
Issue 20728References
Issue 20884References
Issue 21100References
Issue 21101References
Issue 21236References
Issue 21257References
Issue 21354References
Issue 21421References
Issue 21673References
Issue 21742References
Issue 21745References
Issue 21860References
Issue 22093References
Issue 23104References
Issue 23198References
Issue 23247References
Issue 23380References
Issue 23389References
Issue 23525References
Issue 23646References
Issue 23776References
Issue 24237References
Issue 24295References
Issue 24407References
Issue 24759References
Issue 24898References
Issue 24948References
Issue 25086References
Issue 25177References
Issue 25195References
Issue 25369References
Issue 25643References
Issue 25778References
Issue 26062References
Issue 26197References
Issue 26286References
Issue 27221References
Issue 27919References
Issue 28403References
Issue 28615References
Issue 28740References
Issue 29019References
Issue 29021References
Issue 29123References
Issue 29204References
Issue 29293References
Issue 29503References
Issue 29504References
Issue 29743References
Issue 29894References
Issue 29969References
Issue 30095References
Issue 30132References
Issue 30135References
Issue 30187References
Issue 30440References
Issue 30524References
Issue 30686References
Issue 30732References
Issue 30746References
Issue 30814References
Issue 31402References
Issue 31483References
Issue 32377References
Issue 33303References
Issue 33541References
Issue 33989References
Issue 33998References
Issue 34091References
Issue 34420References
Issue 34442References
Issue 34490References
Issue 34493References
Issue 34502References
Issue 34583References
Issue 34689References
Issue 34962References
Issue 35687References
Issue 36028References
Issue 37064References
Issue 37237References
Issue 38288References
Issue 38507References
Issue 38524References
Issue 38559References
Issue 38639References
Issue 38943References
Issue 39285References
Issue 39390References
Issue 39584References
Issue 39650References
Issue 39935References
Issue 40190References
Issue 40412References
Issue 41699References
Issue 42000References
Issue 42070References
Issue 42207References
Issue 43166References
Issue 43207References
Issue 43411References
Issue 43657References
Issue 43751References
Issue 44288References
Issue 44605References
Issue 44681References
Issue 44767References
Issue 44768References
Issue 44876References
Issue 45464References
Issue 45557References
Issue 46279References
Issue 46520References
Issue 46983References
Issue 47112References
Issue 47378References
Issue 47943References
Issue 48416References
Issue 49078References
Issue 49111References
Issue 49871References
Issue 49985References
Issue 50235References
Issue 50397References
Issue 50492References
Issue 50515References
Issue 50570References
Issue 50571References
Issue 50721References
Issue 50854References
Issue 50881References
Issue 50935References
Issue 51133References
Issue 51159References
Issue 51331References
Issue 51332References
Issue 51370References
Issue 51837References
Issue 51949References
Issue 53321References
Issue 53367References
Issue 53432References
Issue 53599References
Issue 54469References
Issue 56267References
Issue 56922References
Issue 57360References
Issue 59255References
Issue 60133References
Issue 60480References
Issue 62079References
Issue 63610References
Footnotes
|
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 " |
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 have adjusted that accordingly. Now I filter by specific keywords using contains:
|
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. Current procedureSet GitHub API Key (we need this because the unauthorized api has low rate limits)Set path to the local copy of your runtime |
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 |
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 Unfortunately, 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 |
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.
However, this is also not done in 2 minutes, so I don't mind support in opening Issues on my behalf. 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, 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. 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)runtime/src/tests/JIT/Methodical/structs/systemvbringup/structpinvoketests.cs
Lines 171 to 175 in f04a242
File
src/coreclr/pal/src/include/pal/mutex.hpp
(File Position 5384-5430)runtime/src/coreclr/pal/src/include/pal/mutex.hpp
Lines 122 to 126 in f04a242
% 0
workaround inMicrosoft.VisualBasic.CompilerServices
#65230File
src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Operators.vb
(File Position 225458-225503)runtime/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Operators.vb
Lines 4592 to 4596 in f04a242
IsEmpty
workaround inSpan
andReadOnlySpan
#64514File
src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs
(File Position 6802-6848)runtime/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs
Lines 154 to 158 in f04a242
File
src/libraries/System.Private.CoreLib/src/System/Span.cs
(File Position 7264-7310)runtime/src/libraries/System.Private.CoreLib/src/System/Span.cs
Lines 159 to 163 in f04a242
File
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs
(File Position 59996-60042)runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs
Lines 1329 to 1333 in f04a242
File
src/libraries/System.ComponentModel.Annotations/tests/System/ComponentModel/DataAnnotations/DataTypeAttributeTests.cs
(File Position 3236-3283)runtime/src/libraries/System.ComponentModel.Annotations/tests/System/ComponentModel/DataAnnotations/DataTypeAttributeTests.cs
Lines 72 to 76 in f04a242
File
src/mono/mono/utils/mono-threads-windows.c
(File Position 4766-4813)runtime/src/mono/mono/utils/mono-threads-windows.c
Lines 111 to 115 in f04a242
File
src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/IsolatedStorageFile.cs
(File Position 1526-1572)runtime/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/IsolatedStorageFile.cs
Lines 38 to 42 in f04a242
ParameterInfo.HasDefaultValue
workaround #65233File
src/libraries/Common/src/Extensions/ParameterDefaultValue/ParameterDefaultValue.netstandard.cs
(File Position 782-828)runtime/src/libraries/Common/src/Extensions/ParameterDefaultValue/ParameterDefaultValue.netstandard.cs
Lines 22 to 26 in f04a242
File
src/libraries/System.Runtime/tests/System/TupleTests.cs
(File Position 11829-11875)runtime/src/libraries/System.Runtime/tests/System/TupleTests.cs
Lines 218 to 222 in f04a242
File
src/libraries/System.ValueTuple/tests/ValueTupleTests.cs
(File Position 12672-12718)runtime/src/libraries/System.ValueTuple/tests/ValueTupleTests.cs
Lines 223 to 227 in f04a242
File
src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs
(File Position 18733-18780)runtime/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs
Lines 401 to 405 in f04a242
File
src/tests/JIT/SIMD/Vector3GetHash.cs
(File Position 379-425)runtime/src/tests/JIT/SIMD/Vector3GetHash.cs
Lines 5 to 9 in f04a242
File
src/libraries/System.Drawing.Common/tests/Imaging/ImageFormatTests.cs
(File Position 5736-5782)runtime/src/libraries/System.Drawing.Common/tests/Imaging/ImageFormatTests.cs
Lines 94 to 98 in f04a242
File
src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs
(File Position 6751-6797)runtime/src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs
Lines 145 to 149 in f04a242
File
src/libraries/System.IO.FileSystem/tests/Directory/EnumerableTests.cs
(File Position 586-633)runtime/src/libraries/System.IO.FileSystem/tests/Directory/EnumerableTests.cs
Lines 14 to 18 in f04a242
File
src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs
(File Position 42960-43006)runtime/src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs
Lines 391 to 395 in f04a242
File
src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/SettingElementTests.cs
(File Position 1871-1917)runtime/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/SettingElementTests.cs
Lines 53 to 57 in f04a242
File
src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs
(File Position 34725-34771)runtime/src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs
Lines 832 to 836 in f04a242
File
src/libraries/System.Private.Xml/tests/XmlReader/Tests/ReaderEncodingTests.cs
(File Position 342-388)runtime/src/libraries/System.Private.Xml/tests/XmlReader/Tests/ReaderEncodingTests.cs
Lines 8 to 12 in f04a242
File
src/libraries/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs
(File Position 5028-5074)runtime/src/libraries/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs
Lines 118 to 122 in f04a242
File
eng/codeOptimization.targets
(File Position 723-769)runtime/eng/codeOptimization.targets
Lines 8 to 12 in f04a242
File
src/coreclr/jit/CMakeLists.txt
(File Position 388-434)runtime/src/coreclr/jit/CMakeLists.txt
Lines 7 to 11 in f04a242
runtime/src/libraries/testPackages/build/packageTest.targets
Lines 30 to 32 in 331823f
File
src/libraries/shims/generated/Directory.Build.props
(File Position 1110-1156)runtime/src/libraries/shims/generated/Directory.Build.props
Lines 23 to 27 in f04a242
File
src/tests/JIT/opt/OSR/tailrecursetry.csproj
(File Position 260-306)runtime/src/tests/JIT/opt/OSR/tailrecursetry.csproj
Lines 5 to 9 in f04a242
File
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/HalfHelpers.cs
(File Position 374-420)runtime/src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/HalfHelpers.cs
Lines 9 to 13 in f04a242
File
src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs
(File Position 3321-3367)runtime/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs
Lines 63 to 67 in f04a242
File
src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs
(File Position 1362-1408)runtime/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs
Lines 33 to 37 in f04a242
File
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.cs
(File Position 2889-2935)runtime/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.cs
Lines 40 to 44 in f04a242
File
src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs
(File Position 22006-22053)runtime/src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs
Lines 511 to 515 in f04a242
Footnotes
Tool source code ↩
The text was updated successfully, but these errors were encountered: