[ https://issues.apache.org/jira/browse/FLINK-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14609742#comment-14609742 ]
ASF GitHub Bot commented on FLINK-2141: --------------------------------------- Github user andralungu commented on the pull request: https://github.com/apache/flink/pull/877#issuecomment-117540482 Hi @shghatge , This PR looks very nice; I added some minor inline comments to make it look even nicer :) One "major" problem: you have an example and a test for it; however, you don't have tests for the GSAConfiguration itself; check https://github.com/apache/flink/blob/master/flink-staging/flink-gelly/src/test/java/org/apache/flink/graph/test/VertexCentricConfigurationITCase.java You need to make sure that given no direction; the iteration works as before and if you give it direction IN or ALL, it behaves as it is supposed to. Apart from that, this looks almost spotless. One other minor thing; when you make the PR, in order to have it uber - clean you should squash your commits. In this case, you have 3 commits, so: git rebase -i HEAD~3 should do the trick Then leave pick for the first commit; squash for the others and let the instructions guide you (Ctrl-x; Y, etc...). Then force push. The common practice afterwards (i.e. after you address my comments here) is to leave it as a new commit (don't squash!). So, first squash these 3, then fix the minor issues and commit again. Awesome work! > Allow GSA's Gather to perform this operation in more than one direction > ----------------------------------------------------------------------- > > Key: FLINK-2141 > URL: https://issues.apache.org/jira/browse/FLINK-2141 > Project: Flink > Issue Type: New Feature > Components: Gelly > Affects Versions: 0.9 > Reporter: Andra Lungu > Assignee: Shivani Ghatge > > For the time being, a vertex only gathers information from its in-edges. > Similarly to the vertex-centric approach, we would like to allow users to > gather data from out and all edges as well. > This property should be set using a setDirection() method. -- This message was sent by Atlassian JIRA (v6.3.4#6332)