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

standardize ^^^ x / !x vvv comments #3208

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Nov 13, 2022

Sometimes we use // ^^^ no workaround / workaround vvv and sometimes // ^^^ no workaround // workaround vvv.
It looks like / is used more often. So I replaced all of // which I had found.
I used //.+// regexp and VS code.
Driven by: #3206 (comment)

@fsb4000 fsb4000 requested a review from a team as a code owner November 13, 2022 17:37
@SuperWig
Copy link
Contributor

Should this PR also settle on double arrows or one on each side i.e.

^^^ _HAS_CXX20 ^^^/ vvv !_HAS_CXX20 vvv
vs
^^^ _HAS_CXX20 / !_HAS_CXX20 vvv

@fsb4000
Copy link
Contributor Author

fsb4000 commented Nov 14, 2022

Sure, and what do you like more?

@SuperWig
Copy link
Contributor

My vote would for the second. I believe this is also part of #351

@CaseyCarter CaseyCarter added the documentation Related to documentation or comments label Nov 14, 2022
@strega-nil-ms
Copy link
Contributor

I'll take a look at this once we discuss #351

Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Might as well standardize the location of the ^^^ and vvv as well; also do that through the rest of the file (probably should've just done one suggestion and then said that... sorry...)

stl/inc/bitset Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/execution Outdated Show resolved Hide resolved
stl/inc/execution Outdated Show resolved Hide resolved
stl/inc/list Outdated Show resolved Hide resolved
stl/inc/span Outdated Show resolved Hide resolved
stl/inc/span Outdated Show resolved Hide resolved
stl/inc/string Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
@strega-nil-ms strega-nil-ms changed the title replace // => / in comments standardize ^^^ x / !x vvv comments Dec 1, 2022
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks for making this more consistent and easier to read! The changes to <xcharconv_ryu.h> are fine (they're in additions to the upstream code and don't represent unnecessary divergence).

@StephanTLavavej StephanTLavavej self-assigned this Dec 5, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit be29af2 into microsoft:main Dec 6, 2022
@StephanTLavavej
Copy link
Member

Thanks again for this consistency pass! 😸 🎉 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation or comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants