paulirwin commented on code in PR #1: URL: https://github.com/apache/lucenenet-codeanalysis-dev/pull/1#discussion_r1993656488
########## src/Lucene.Net.CodeAnalysis.Dev/AnalyzerReleases.Unshipped.md: ########## @@ -0,0 +1,9 @@ +### New Rules Review Comment: First, it's worth pointing out that there are two possible problems being conflated here: 1. Two (or more) contributors working in parallel and needing to reserve IDs so they are unique, and don't need to rework their PRs 2. Ensuring that IDs in the project are unique within the working directory of the project at build time, and that they match the categories they're expected to be in These are not the same problems. Microsoft's solution only solves the latter; it does not appear like they are solving the former with this. If you look at [the history of that DiagnosticCategoryAndIdRanges.txt file](https://github.com/dotnet/roslyn-analyzers/commits/main/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt), you can see that it is only incremented as part of the PRs. That means that it alone does not solve the ID uniqueness/reservation problem once it is integrated, or having to avoid PR rework. If another contributor is working on one in parallel, this file (how they use it) does not provide global uniqueness guarantees, because it is only incremented in main when it is merged (de facto, anyways, based on historical evidence). The contributor might have to update their PR to change their IDs if another person was working on the same ID. Their process just ensures the file is correct and the IDs are unique after already being integrated; it doesn't do anything to help the "stepping on each others' toes" problem before integration. Both are valid problems, it should be noted. My proposed solution does solve the former problem of reservation (as best as is possible in git), with a simpler solution. And by solving the former problem, it solves a Pareto-optimal amount of the second problem, because if you're reserving IDs in advance of developing the analyzers, then by definition (assuming that reservation was done correctly) they would be unique within the working directory of the project (either in your PR feature branch, or integrated into main) too. We would use the same file/format, but we would treat what is in main as the source of truth. You would reserve an ID (or multiple) in advance by committing a change to that file to bump the ID. (If during development you discover you need more IDs, no problem, just bump the file again in main.) This way, you would get a merge conflict if you're trying to bump an older version of main, and would have to bump it again to a new ID. Then, with your ID(s) in hand, you go and submit the PR, which should be conflict-free ID-wise from any other PRs going on at the same time (that have also previously reserved theirs in main). Also, something I left out from my prior message. If someone is not a committer to the repo, they can just request in the issue for another committer to reserve one or more IDs for them, then someone like me would be happy to do that, commit the change to main, and let the contributor know which IDs were reserved to them. No problem. If we do not do this, then we still have the first problem, even if we write a great meta-analyzer. We can add one later (I think we should split that out as a separate issue), and we can manually ensure compliance in PR review in the short term. The meta-analyzer will be some good process-hardening that will be nice for the long-term success of this project but is not needed right now (I'd suggest we do it between beta 18 and GA to not hold up the beta 18 release over this). Again, I'm not saying to not use the file, quite the opposite as noted above. Just that we don't need the meta-analyzer at this time. This would be a simple solution that starts us on the path to that, solves most of the big uniqueness problems in advance, while letting this get hardened in an automated way later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
