Skip to content

_MSVC_STL_UPDATE changed of current month #1801

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
Merged
StephanTLavavej merged 1 commit into from
Apr 6, 2021
Merged

_MSVC_STL_UPDATE changed of current month #1801

StephanTLavavej merged 1 commit into from
Apr 6, 2021

Conversation

@2002Bishwajeet
Copy link
Contributor

@2002Bishwajeet 2002Bishwajeet commented Apr 2, 2021

Fixed issue #1799

@2002Bishwajeet 2002Bishwajeet requested a review from a team as a code owner April 2, 2021 18:29
@mnatsuhara mnatsuhara linked an issue Apr 2, 2021 that may be closed by this pull request
<yvals_core.h>: Update _MSVC_STL_UPDATE to April 2021 #1799
Closed
mnatsuhara
mnatsuhara approved these changes Apr 2, 2021
View reviewed changes
Copy link
Contributor

@mnatsuhara mnatsuhara 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 added the enhancement Something can be improved label Apr 2, 2021
CaseyCarter
CaseyCarter approved these changes Apr 2, 2021
View reviewed changes
Copy link
Contributor

@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.

All good. Congratulations on your first STL PR!

@2002Bishwajeet
Copy link
Contributor Author

2002Bishwajeet commented Apr 2, 2021

Thank You so much for approving them😊. I would like to contribute more, can you guide me further on what to do next??

@StephanTLavavej StephanTLavavej self-assigned this Apr 2, 2021
@StephanTLavavej StephanTLavavej merged commit b88c580 into microsoft:main Apr 6, 2021
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Apr 6, 2021

Thanks for updating this version macro, and congratulations on your first microsoft/STL commit! 🎉 😺

As @mnatsuhara mentioned on Discord, the maintainer team is busy with finishing C++20 so our ability to provide guidance is limited at the moment. After briefly looking over our issues list, I think that further PRs to improve #206 could be suitable. Instead of trying to apply _NODISCARD to the entire STL (which would be a ton of work), you can pick just one header (or a small handful) and look for opportunities to apply _NODISCARD following our criteria. #1474 is an excellent example of such a PR.

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

Reviewers

3 more reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@fsb4000 fsb4000 fsb4000 approved these changes

@mnatsuhara mnatsuhara mnatsuhara approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

enhancement Something can be improved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

<yvals_core.h>: Update _MSVC_STL_UPDATE to April 2021

5 participants

@2002Bishwajeet @StephanTLavavej @CaseyCarter @fsb4000 @mnatsuhara