Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Adding test to catch zero length subgraph bug #84

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

adstraw
Copy link
Contributor

@adstraw adstraw commented Nov 29, 2017

some diff noise in test_ngraph.h from clang-format, the pertinent change is at line 66

Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

LGTM. Targets a bug that we've fixed.

@adstraw
Copy link
Contributor Author

adstraw commented Nov 30, 2017

accidentally removed and re-added @mbrookhart so his checkmark doesn't show up anymore. @ransford2011 or @louisfeng can you take a quick look?

Copy link
Contributor

@louisfeng louisfeng left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

auto var_node = std::make_shared<VariableNode>(test_input, "test_input");
test_inputs.push_back(var_node);
};

virtual void TearDown() {};
virtual void TearDown(){};
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised clang format removed a space in this case and kept a space in other cases.

@@ -35,8 +35,7 @@ class NGRAPH_NODE : public ::testing::Test {
};

class NGRAPH_GRAPH : public ::testing::Test {
protected:

protected:
static bool isop(NodePtr s) { return (s->type_ == NodeType::kOp); };
Copy link
Contributor

Choose a reason for hiding this comment

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

For example, here it didn't touch the space. Just wondering out loud.

Copy link
Contributor

Choose a reason for hiding this comment

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

:( Who knows.

@mbrookhart mbrookhart merged commit 0ceed8e into ngraph-integration-dev Nov 30, 2017
@mbrookhart mbrookhart deleted the adstraw/ngraph-graph-test branch November 30, 2017 22:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants