Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jan 17, 2020

Came across this while reviewing something else. See ConvertToVirtualBaseInstruction a handful of lines below the change for proof that a ConvertToBaseInstruction is not necessarily non-virtual.

@geoffw0 geoffw0 added the C++ label Jan 17, 2020
@geoffw0 geoffw0 requested a review from a team as a code owner January 17, 2020 16:11
Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

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

LGTM with proposed additional changes.

Additional corrections.

Co-Authored-By: Dave Bartolomeo <dbartol@github.com>
dbartol
dbartol previously approved these changes Jan 17, 2020
Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

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

LGTM

Except you need to run sync-identical-files to propagate the changes to C#.

@geoffw0 geoffw0 requested a review from a team as a code owner January 23, 2020 10:23
@geoffw0
Copy link
Contributor Author

geoffw0 commented Jan 23, 2020

Thanks, I've never used that script before - done.

@geoffw0 geoffw0 removed the request for review from a team January 23, 2020 10:23
@jbj jbj merged commit 386e8e8 into github:master Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants