Skip to content

Remove redundant uses of virtual keyword#2069

Merged
StephanTLavavej merged 6 commits intomicrosoft:mainfrom
SuperWig:override
Aug 6, 2021
Merged

Remove redundant uses of virtual keyword#2069
StephanTLavavej merged 6 commits intomicrosoft:mainfrom
SuperWig:override

Conversation

@SuperWig
Copy link
Contributor

@SuperWig SuperWig commented Jul 21, 2021

Fixes #303.
Fixes #207.

Opening as draft due to "decision-needed" tag, though there wasn't anyone opposing. The ones that go onto a new line do make it harder to notice the override.

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Jul 21, 2021
@StephanTLavavej
Copy link
Member

We've decided that this is indeed desirable - thanks! 😸

@AlexGuteniev
Copy link
Contributor

Apparently this will resolve #207 as well

@SuperWig SuperWig marked this pull request as ready for review July 22, 2021 07:44
@SuperWig SuperWig requested a review from a team as a code owner July 22, 2021 07:44
@StephanTLavavej StephanTLavavej self-assigned this Jul 28, 2021
@StephanTLavavej
Copy link
Member

Looks great, thanks! I've pushed a merge with main (no conflicts, including stealth clang-format conflicts) and I found no additional occurrences in product code that need to be changed. The fact that we were missing virtual entirely in some locations was 🙀 - thank you for cleaning everything up to say override. 😻

@StephanTLavavej StephanTLavavej merged commit c76042b into microsoft:main Aug 6, 2021
@StephanTLavavej
Copy link
Member

Thanks for this comprehensive cleanup! 🧹 😻 🚀

@SuperWig SuperWig deleted the override branch August 6, 2021 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

STL: Should we avoid repeating virtual for override functions? STL: Finish marking virtual functions as override

4 participants