[SPARK-15057][GRAPHX] Remove stale TODO comment for making enum in GraphGenerators

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR removes a stale TODO comment in GraphGenerators.scala

How was this patch tested?

Just comment removed.

[SparkQA]

Test build #57522 has finished for PR 12839 at commit 8ef3f3b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

I disagree with this. Quadrants are conventionally referred to by number (really, 1-4 not 0-3, but hey) rather than a letter. This is a private API anyway. I don't think this is improved with an enum.

[dongjoon-hyun]

Hi, @srowen . Thank you for review!
Then, may I just update this PR by simply removing the TODO comments?

// TODO(crankshaw) turn result into an enum (or case class for pattern matching}

Actually what I wanted to do was removing that long-lasting TODO comment. :)

[srowen]

I personally favor trivially resolving this by removing the TODO. I don't recognize the ABCD notation for quadrants anyway.

[dongjoon-hyun]

Oh, actually, the ABCD notation comes from the description of that function.
https://github.com/dongjoon-hyun/spark/blob/SPARK-15057/graphx/src/main/scala/org/apache/spark/graphx/util/GraphGenerators.scala#L181

   * <pre>
   *
   *          dst ->
   * (x,y) ***************  _
   *       |      |      |  |
   *       |  a   |  b   |  |
   *  src  |      |      |  |
   *   |   ***************  | T
   *  \|/  |      |      |  |
   *       |   c  |   d  |  |
   *       |      |      |  |
   *       ***************  -
   * </pre>

[srowen]

That makes more sense, OK. It's not referring to the usual quadrant I - IV in the plane. I think that's still not the recommended way to make enums in Scala (?) and may simply not be worth changing now, IMHO.

[dongjoon-hyun]

Sure. I just thought enum improves the return and match exhaustiveness, but I agree with your opinion, too.
I'll update this PR for just removing TODO.

[dongjoon-hyun]

Thank you for making decision for this PR, @srowen .
I updated the PR and JIRA accordingly. Now, this just removes comment.

[dongjoon-hyun]

Could you merge this PR, @srowen ?

[SparkQA]

Test build #57547 has finished for PR 12839 at commit dd9b008.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

OK, merged to master

[dongjoon-hyun]

Thank you, @srowen .

[rxin]

Since this is very low risk, I'm going to cherry-pick this in branch-2.0 too to minimize the diff.

[dongjoon-hyun]

Thank you, @rxin . :)