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.

Sign up for GitHub

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

Jump to bottom

<format>: parameter name consistency: _First/_Last vs _Begin/_End #1951

Merged
merged 6 commits into from
Jun 12, 2021
Merged

<format>: parameter name consistency: _First/_Last vs _Begin/_End #1951

merged 6 commits into from
Jun 12, 2021

Conversation

mghalayini
Copy link
Contributor

This PR enhances #1947. All _Begin and _End parameters have been changed to _First and _Last, respectively. There is also a function on line 2081 (_Write_separated_integer) which uses the parameter names _Start and _End, so I edited that function too conforming to the standard parameter names.

@mghalayini mghalayini requested a review from a team as a code owner June 2, 2021 12:26
@ghost
Copy link

ghost commented Jun 2, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@mghalayini
Copy link
Contributor Author

Sorry for causing a mess there, I was having some trouble using the clang-format tool for the first time.

Again, all I did was change the _Begin and _End parameters to _First and _Last, respectively. Everything else was just trying to properly format the code using clang-format.

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Jun 2, 2021
@CaseyCarter CaseyCarter linked an issue Jun 2, 2021 that may be closed by this pull request
<format>: parameter name consistency: _First/_Last vs _Begin/_End #1947
Closed
CaseyCarter
CaseyCarter requested changes Jun 2, 2021
View reviewed changes
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

This is all great. Could we please change the names of the local _Begin and _End variables in _Parse_format_string on lines 946-983 similarly?

@CaseyCarter CaseyCarter self-assigned this Jun 2, 2021
@mghalayini
Copy link
Contributor Author

@CaseyCarter Done.

CaseyCarter
CaseyCarter approved these changes Jun 3, 2021
View reviewed changes
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Looks great!

@CaseyCarter CaseyCarter removed their assignment Jun 3, 2021
sam20908
sam20908 approved these changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@sam20908 sam20908 left a comment

Choose a reason for hiding this comment

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

LGTM

@StephanTLavavej StephanTLavavej added the format C++20/23 format label Jun 10, 2021
barcharcraz
barcharcraz approved these changes Jun 10, 2021
View reviewed changes
Copy link
Member

@barcharcraz barcharcraz left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

@StephanTLavavej StephanTLavavej self-assigned this Jun 11, 2021
@StephanTLavavej StephanTLavavej merged commit 39e7778 into microsoft:main Jun 12, 2021
@StephanTLavavej
Copy link
Member

Thanks for improving codebase consistency - and congratulations on your first microsoft/STL commit! 🎉 😸 🚀 This will ship in VS 2022 17.0 Preview 2.

@mghalayini mghalayini deleted the parameter-consistency branch June 12, 2021 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sam20908 sam20908 sam20908 approved these changes

@barcharcraz barcharcraz barcharcraz approved these changes

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
enhancement Something can be improved format C++20/23 format
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<format>: parameter name consistency: _First/_Last vs _Begin/_End
5 participants
@mghalayini @StephanTLavavej @CaseyCarter @barcharcraz @sam20908