GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-67341: fix comment about windows link stat mapping by moreal · Pull Request #136049 · python/cpython

Conversation

@moreal

@moreal moreal commented Jun 27, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

// Windows doesn't define S_IFLNK, but posixmodule.c maps

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 to fileutils.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.

  • Issue: fstat64 required on Windows #67341

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added the awaiting review label Jun 27, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 27, 2025
fstat64 required on Windows #67341
Closed
@moreal
moreal force-pushed the update-comment-latest branch from 46c1b85 to b64d3ae Compare June 27, 2025 16:32
@moreal moreal changed the title gh-67341: Update a comment in fileutils.h to latest gh-67341: fix comment about windows link stat mapping Jun 27, 2025
@picnixz picnixz added the skip news label Jun 27, 2025
@ZeroIntensity

ZeroIntensity commented Jun 27, 2025

Copy link
Copy Markdown
Member

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!

moreal reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@moreal

moreal commented Jun 28, 2025

Copy link
Copy Markdown
Contributor Author

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!

ZeroIntensity reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@moreal
moreal force-pushed the update-comment-latest branch from b64d3ae to 131d398 Compare June 28, 2025 16:19
@moreal
Update a comment in fileutils.h to latest
691d76e
@moreal
moreal force-pushed the update-comment-latest branch from 131d398 to 691d76e Compare June 30, 2025 07:45
@ZeroIntensity

ZeroIntensity commented Jun 30, 2025

Copy link
Copy Markdown
Member

FYI, no need to force push. We squash at the end.

moreal reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

moreal added 2 commits July 1, 2025 17:20
@moreal
Merge branch 'main' into update-comment-latest
19d797f
@moreal
Merge branch 'main' into update-comment-latest
481927b
picnixz
picnixz approved these changes Jul 14, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 14, 2025
@picnixz

picnixz commented Jul 14, 2025

Copy link
Copy Markdown
Member

Yeah I've accepted the other PR because it's annoying when a comment is wrong especially when it's about defining stuff.

moreal reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz enabled auto-merge (squash) July 14, 2025 07:39
@picnixz
picnixz merged commit cd3e7ac into python:main Jul 14, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 14, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@moreal @taegyunkim
pythongh-67341: fix a typo in Include/fileutils.h (python#136049)
d2ed291
`IO_REPARSE_TAG_SYMLINK` is mapped to `S_IFLNK` in `fileutils.c`, not in `posixmodule.c`
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@moreal @Agent-Hellboy
pythongh-67341: fix a typo in Include/fileutils.h (python#136049)
e19d952
`IO_REPARSE_TAG_SYMLINK` is mapped to `S_IFLNK` in `fileutils.c`, not in `posixmodule.c`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fstat64 required on Windows

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@moreal @ZeroIntensity @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.