-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
gh-57943: disambiguate issue number in os._fwalk
comment
#137327
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
Conversation
We prefer GH issues rather than BPO numbers but I would suggest something like: "See gh-XXXX" or "See https://github.com/..." where with links I can click on them from the IDE. |
os._fwalk
comment
I think it's an improvement but at the same time it's a borderline cosmetic. But since such comments are meant to be read, I would indeed consider it a real improvement. @AA-Turner Are you ok with such change? I don't want to encourage cosmetic changes but I feel that this one is half-cosmetic half-improvement. Or maybe you would prefer a PR that edits the BPO issue numbers in the code base? (not everyone knows what BPO is actually and it doesn't necessarily come up directly when googling) |
I agree, which is why I was hesitant to open the PR. In order to track down the issue I opened up github issues had to understand it was the wrong one before looking for it in bpo. Perhaps I should have gone to bpo first. |
There are perhaps thousands of references to the bpo issues there. I do not see a point in only changing this one. |
Arf. Ok. If there are that many references, then it's better to change them in one go if we do it. |
I can close this PR and log an issue. |
Who would review such large change? We can also rewrite commit messages and old discussions on the tracker and in mailing lists. Just remember that there are two different numbering systems. And if not specified explicitly, the issue number is most likely refers to BPO. |
Mmh, ok. I wouldn't have thought that there were that many occurrences. Let's forget about updating the references then. However, it would make sense to indicate that BPO historically existed, and for that reason, if contributors want to references issues, they should either add a full link (like me) or In summary: let's do nothing in CPython's code itself. Sorry for giving false merging hope. |
Thanks for the reviews ^_^ |
Reference to bpo issue: https://bugs.python.org/issue13734 made in an inline comment, I've added the
bpo-
prefix andgh
reference to disambiguate to which issue tracker the reference is making.I'm not sure if it's preferred to get rid of
bpo
references and keep onlygh
references. 🤷🏼gh ref: #57943