Skip to content
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

[Documentation] Fix lowercase DLL names and standardize uppercase X for variable version number #2089

Merged
merged 3 commits into from
Jan 15, 2024

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Jul 24, 2023

These are documentation-only changes, and more of a suggestion. Has some overlap with #1990 so it'll reduce changes there.

Standardised:

  • uppercase X for unknown/variable version number
  • spacing in Python with version
  • specifying that we're talking about Python for "floating/magic" version numbers
  • Also made dll names match their actual casing

Example changes:

  • IsActuallyLowercase.dll --> isactuallylowercase.dll
  • 3.x -> Python 3 (or py3k)
  • python 3 -> Python 3
  • Python3 -> Python 3
  • Python 3.x -> Python 3
  • pythonxx.dll -> pythonXX.dll
  • "eg: Python 3.7.2" -> "eg: Python X.X.X"

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 12, 2024

@mhammond (and @vernondcole since it touches adodbapi documentation) thoughts? The Python version mentions would especially have made it much easier to search for outdated code in previous PRs. And help clarify certain comments.

@mhammond
Copy link
Owner

I'm fairly ambivalent about much of this. In particular, some of the DLL renames are moving away from what's on the file-system to all lower-case, and pythoncomxx -> pythoncomXX and python 2.x -> python 2, or Python -> python` all seem marginal value at best.

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 12, 2024

some of the DLL renames are moving away from what's on the file-system to all lower-case

That's curious, on my machine they are all lowercase. Is this not always the case for those DLLs?
image

@mhammond
Copy link
Owner

heh - yeah, I think I might have actually changed that semi-recently for our DLLs. I recall mapi used to have the mixed-case name somewhere, but that was long long ago, so I can't argue with evidence that I'm wrong about that, sorry.

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 12, 2024

No worries, avoiding this kind of confusion about actual file names is a small reason why I'm opening this PR in the first place.

With all the recent code cleanup in preparation for #2102 , I figured may as well do a pass of comments/doc cleanup for nitpicky things I've noticed along the way. I don't think you need to hard enforce those in the future or whatever. But rather may as well do it in a sweeping change.

I think that accurate file names and mentioning "Python" (or py2k/py3k) along a standalone version number does have some (although slight) value. Namely making it easier to find areas of code that can be simplified when dropping support for a Python version. Or understanding at a glance, with certainty, that the version number is related to Python, not something else.

Dropping .x or the capitalization of xx vs XX, python vs Python was just to standardize it across the code base. But can definitely be reverted if deemed too noisy even for this type of PR.


Note, looking back at file changes, there is one code change, in com/win32comext/taskscheduler/test/test_addtask_2.py, that might come from a comment/conversation in a different PR?

Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - and yeah, the add_task changes already landed.

@mhammond mhammond merged commit 0bbc2bc into mhammond:main Jan 15, 2024
16 checks passed
@Avasam Avasam deleted the dll-name-and-XX branch January 15, 2024 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants