[ 
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)

Reply via email to