Github user andralungu commented on the pull request:
https://github.com/apache/flink/pull/807#issuecomment-110070080
Hi @shghatge ,
This very nice for a first PR and I am happy to see that you followed my
guidelines :)
I left a set of comments in-line.
Apart from those:
- the difference method can be simplified. You don't need to filterOnEdges.
Have a closer look at removeVertices. Imagine what happens if you remove a
vertex, the edge will also have to be removed. You cannot leave an edge with
just the source or the target vertex trailing.
- I think you forgot to add the corner case test for an input graph which
does not have common vertices with the first one. I know you wrote it :)
- Finally, if you have a look at the Travis build here, it failed because
you are indenting with spaces instead of tabs. You should play a bit with your
IntelliJ settings. No worries! This is a rookie mistake, we all did it at
first. To check everything is okay, just do a cd flink-staging/flink-gelly and
then mvn verify. After it says build success, we're good to go.
Rebase and update the PR.
If you have questions, I'll be more than happy to answer them! Nice job!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
