-
Notifications
You must be signed in to change notification settings - Fork 699
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.
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
Ported SparseLengthsSumTest to EE2 #3309
Conversation
c71857e
to
a7b4462
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcatron has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
a7b4462
to
00661d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcatron has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
00661d6
to
7632207
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcatron has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
class SparseLengthsSum : public BackendTest {}; | ||
class SparseLengthsSum : public BackendTest { | ||
public: | ||
~SparseLengthsSum() override{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dtor in the base class needs to be virtual.
Summary:Ported SparseLengthsSumTest to EE2
Documentation:
Progress on #3239
Test Plan: Verify everything builds, Verify SparseLengthsSumTest runs and passes. (I'm not sure this is actually tested in CI. Verified to run on my laptop.)