-
Notifications
You must be signed in to change notification settings - Fork 967
Update zc-conformance.md for accuracy and consistency #4665
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
Learn Build status updates of commit d3fafb0: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Thank you for your contribution. Would you take a moment to sign the Contributor License Agreement (CLA)? After the CLA is signed, someone can review your pull request. Thanks! #label:"aq-pr-triaged" |
Apologies for the delay. I'm still waiting on my company's approval to sign the CLA. |
| [`/Zc:char8_t`](zc-char8-t.md) | Enable or disable C++20 native `u8` literal support as `const char8_t` (off by default, except under **`/std:c++20`**). | | ||
| [`/Zc:alignedNew[-]`](zc-alignednew.md) | Enable C++17 over-aligned dynamic allocation (implied by **`/std:c++17`** or later). | | ||
| [`/Zc:auto[-]`](zc-auto-deduce-variable-type.md) | Enforce the new Standard C++ meaning for **`auto`** (on by default). | | ||
| [`/Zc:char8_t[-]`](zc-char8-t.md) | Enable or disable C++20 native `u8` literal support as `const char8_t` (implied by **`/std:c++20`** or later). | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I sometimes like the 'implied by' approach, but it doesn't mean the same thing as the text it replaces. It indicates that the switch may be on when another switch is specified, but it loses the clarity that it is off in all other cases. It replaces a statement that is true in all cases with a more ambiguous one that it is true in a specific case. I prefer the wording for /Zc:char8_t, for example. I'd rather not take the 'implied by' changes unless they add the extra clarity that they are off, otherwise. Thank you for adding the optional [-] notes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My goal was to make these parentheticals consistent, which improves readability. I chose the "implied by" form because it was terser. But I don't mind changing it over to the form on this line.
Some cases are a bit different than others. If you still don't like the wording, let me know and I'll update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced "implied by" everywhere with the form on line 29.
I also made another tweak to the twoPhase row. That one is a trickier special case. I hope the new wording is clearer. I think the original wording was prone to misinterpretation.
Learn Build status updates of commit cc3c5f8: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Changes assume accuracy of the individual option doc pages * Update default values per individual option docs * Use consistent wording and formatting for default values * Mention [-] syntax where applicable
cc3c5f8
to
db05a2d
Compare
Learn Build status updates of commit db05a2d: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like what you did there. Thank you!
#sign-off |
@balagansky-work - No problem! After you've signed the CLA, this PR can be merged. Thanks! #hold-off |
Unfortunately, my company's process for signing this CLA is time consuming and costly relative to the trivial nature of this change. I notice in https://github.com/MicrosoftDocs/cpp-docs/blob/main/CONTRIBUTING.md the possibility of waiving the CLA signature requirement for minor changes. Would this PR perhaps be a candidate for waiving the CLA requirement? If not, I will need to close the PR. Thank you and apologies for the inconvenience. |
@balagansky-work - Thanks for letting us know. The CLA completion is required for this PR. @TylerMSFT - Would you like to pull these changes into the private repo and merge? |
Side Question: Is there some threshold that triggers the CLA requirement on a pull request? Or does it happen on any pull request for someone without a CLA on record? (I have one already so I can't tell if there is any difference.) @Court72 your comments seem contradictory. Is that how a decision that a change is minor works? |
@orcmid - That's correct! From the documentation @balagansky-work linked: "When your PR is created, it is classified by a CLA bot. If the change is trivial (for example, you fixed a typo), then the PR is labeled with CLA-not-required. Otherwise, it's classified as CLA-required. Once you signed the CLA, the current and all future pull requests are labeled as CLA-signed." |
I don't want to lose the work that @balagansky-work has put in here. I'll port the changes and check in under my account. |
@balagansky-work, I have taken your changes and put them in a private repo and integrated them into the docs. They'll be live by tomorrow. |
Changes assume accuracy of the individual option doc pages