Skip to content

LWG-3833 Remove specialization template<size_t N> struct formatter<const charT[N], charT> - #3477

Merged
Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
mlemacio:main
Feb 23, 2023
Merged

LWG-3833 Remove specialization template<size_t N> struct formatter<const charT[N], charT>#3477
Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
mlemacio:main

Conversation

@mlemacio

@mlemacio mlemacio commented Feb 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Remove specialization template<size_t N> struct formatter<const charT[N], charT>
Fixes #3426

@mlemacio
Implement LWG-3833
384086c 
Remove template specialization
@mlemacio
mlemacio requested a review from a team as a code owner February 17, 2023 21:48
@mlemacio mlemacio mentioned this pull request Feb 17, 2023
Closed
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added LWG Library Working Group issue format C++20/23 format labels Feb 17, 2023
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Feb 17, 2023

Copy link
Copy Markdown
Member

Thanks for the PR! Can you edit your PR description to add "Fixes #3426."? This is special syntax for GitHub that will link the issue to your PR. (It only takes effect in the PR description, not the title, and not later comments where issues can only be "mentioned".)

@mlemacio

mlemacio commented Feb 17, 2023

Copy link
Copy Markdown
Contributor Author

Added to PR description

strega-nil-ms
nicole mazzuca (strega-nil-ms) approved these changes Feb 17, 2023
View reviewed changes

@strega-nil-ms nicole mazzuca (strega-nil-ms) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@mlemacio

mlemacio commented Feb 17, 2023

Copy link
Copy Markdown
Contributor Author

Yay! Are there any recommended resources on how to get more knowledgeable on the ecosystem so that I may contribute more? I consider myself a fairly seasoned user of C++ but this has been my first foray into open source.

@fsb4000

Igor Zhukov (fsb4000) commented Feb 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Are there any recommended resources on how to get more knowledgeable on the ecosystem so that I may contribute more?

You can watch the code reviews and notice what was wrong and don't do that in your PRs: https://www.youtube.com/playlist?list=PLReL099Y5nRffygixwNJhENbYxhuJL6Ei

@mlemacio

mlemacio commented Feb 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Igor Zhukov (@fsb4000) Just watched my first one in the playlist. Had a good time doing do. Thanks!

@mlemacio

mlemacio commented Feb 18, 2023

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@fsb4000

Igor Zhukov (fsb4000) commented Feb 18, 2023

Copy link
Copy Markdown
Contributor

Something bad happened.
I think the correct commit is 384086c

CaseyCarter
Casey Carter (CaseyCarter) suggested changes Feb 18, 2023
View reviewed changes

@CaseyCarter Casey Carter (CaseyCarter) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It appears that you've force-pushed the changes from your other PR atop this one.

@mlemacio

mlemacio commented Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

Sorry about that, should be fixed.

@CaseyCarter Casey Carter (CaseyCarter) mentioned this pull request Feb 21, 2023
Closed
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Feb 22, 2023

Copy link
Copy Markdown
Member

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

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit be1ac63 into microsoft:main Feb 23, 2023
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Feb 23, 2023

Copy link
Copy Markdown
Member

Thanks for implementing this LWG issue resolution, and congratulations on your first microsoft/STL commit! 😻 🚀 🎉

This will ship in VS 2022 17.7 Preview 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) StephanTLavavej approved these changes
+2 more reviewers
@CaseyCarter Casey Carter (CaseyCarter) CaseyCarter approved these changes
@strega-nil-ms nicole mazzuca (strega-nil-ms) strega-nil-ms approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej)

Labels

format C++20/23 format LWG Library Working Group issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

LWG-3833 Remove specialization template<size_t N> struct formatter<const charT[N], charT>

5 participants

@mlemacio @StephanTLavavej @fsb4000 @CaseyCarter @strega-nil-ms