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

Update WINDOWS.md #24242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update WINDOWS.md #24242

wants to merge 1 commit into from

Conversation

ha1215
Copy link

@ha1215 ha1215 commented Apr 23, 2024

The possessive form of "Windows" has been updated from "Windows's" to "Windows'".

The function call "a poll(2) call" has been specified as "a poll(2) system call" for clarity.

The phrase "and supposed" has been corrected to "and was supposed" to improve sentence structure.

The phrase "However Microsoft has" now includes a comma, revised to "However, Microsoft has," to enhance readability.

The statement "Supporting these is a pain" has been adjusted to "Supporting these can be a pain" to better convey potential variability in user experience.

CLA: trivial

This is my PR requests for the Software Engineering Assignment with @bbbrumley.

The possessive form of "Windows" has been updated from "Windows's" to "Windows'".

The function call "a poll(2) call" has been specified as "a poll(2) system call" for clarity.

The phrase "and supposed" has been corrected to "and was supposed" to improve sentence structure.

The phrase "However Microsoft has" now includes a comma, revised to "However, Microsoft has," to enhance readability.

The statement "Supporting these is a pain" has been adjusted to "Supporting these can be a pain" to better convey potential variability in user experience.
@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Apr 23, 2024
@@ -2,11 +2,11 @@ Windows-related issues
======================

Supporting Windows introduces some complications due to some "fun" peculiarities
of Windows's socket API.
of Windows socket API.
Copy link
Contributor

Choose a reason for hiding this comment

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

The possessive form of "Windows" has been updated from "Windows's" to "Windows'".

One too many---the apostrophe disappeared :D

Copy link
Member

Choose a reason for hiding this comment

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

IMO OK without the apostrophe.

Copy link
Member

Choose a reason for hiding this comment

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

I also find Windows socket API is already very understandable. Just like we usually use the term Linux/Unix socket programming instead of Linux's socket...

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Windows' - apostrophe at the end. It's hard to see in @bbbrumley's comment

Copy link
Contributor

Choose a reason for hiding this comment

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

It should either be ... peculiarities of the Windows socket API or ... peculiarities of Windows' socket API. I think the former would be better

@bbbrumley
Copy link
Contributor

Thanks! Let's see what the maintainers say. For Windows's vs Windows', did you check the rest of the lib for consistency?

This looks fine to me, but the maintainers are still gonna want the CLA line in the actual commit message; see the documentation here, particularly the part about "To amend a missing .."

@t8m t8m added branch: master Merge to master branch triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly tests: exempted The PR is exempt from requirements for testing labels Apr 23, 2024
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

No strong view on Window's vs Windows. Otherwise seems ok.

I consider this trivial. Please add the "CLA: trivial" line to your commit.

@mattcaswell mattcaswell added the approval: review pending This pull request needs review by a committer label Apr 24, 2024
Copy link
Contributor

@shahsb shahsb left a comment

Choose a reason for hiding this comment

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

Okay with trivial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch hold: cla required The contributor needs to submit a license agreement tests: exempted The PR is exempt from requirements for testing triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants