-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
gh-67341: fix comment about windows link stat mapping #136049
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
46c1b85
to
b64d3ae
Compare
Hello, thanks for your interest in contributing! I see that your prior PR, #136027, was accepted, but please don't use that as a green light to make several PRs fixing comment typos. These generally fall under the umbrella of "cosmetic changes", which we try to avoid. That said, this PR is already here, so we might as well use it. Are there any other comment fixes you would like to make? Let's do it all in one change, rather than many tiny changes; it's easier for us to handle from a review stance. After this one, please try to avoid sole comment changes. Thanks! |
Hello, thank you for letting me know about that policy! I apologize for taking up the maintainers' attention. I was referencing the CPython source code while working on other tasks, and these were comment errors I discovered in that process, so there are no additional fixes to add. The reason it became 2 PRs is because I submitted them as I found them while reading through the source code. Next time, even if I discover such issues (cosmetic changes), I'll just understand and move on without submitting fixes. Thanks! |
b64d3ae
to
131d398
Compare
131d398
to
691d76e
Compare
FYI, no need to force push. We squash at the end. |
Yeah I've accepted the other PR because it's annoying when a comment is wrong especially when it's about defining stuff. |
`IO_REPARSE_TAG_SYMLINK` is mapped to `S_IFLNK` in `fileutils.c`, not in `posixmodule.c`
The above comment is written in 99d6135#diff-b5b7e4f5599916b2018e911ef423f9d3fd46cb71268e1a73c74a1c9ae2cb6791 commit, and
posixmodule.c
included such logic at that time.The commit f2f373f which resolves #67341, moved the logic from
posixmodule.c
tofileutils.c
.This pull request includes only trivial changes but I've attached the GitHub issue number as a prefix, which I believe corresponds to Issue 23152 mentioned in the commit message. At the time, the contributors were using a separate tool outside of GitHub, so I thought it would be appropriate to match it with the bpo number listed in the issue description. Please let me know if there are any mistakes! Thank you.