[MINOR] Typo fixes #17434
[MINOR] Typo fixes #17434
Conversation
|
Could you batch these together if you're fixing up some miscellaneous doc typos? and have a look through for more? as we've mentioned a few times, it's just not efficient to review one-liners. |
|
Test build #75235 has finished for PR 17434 at commit
|
jaceklaskowski
force-pushed
the
from_json-typo
branch
from
3b38503 to
5016af1
Compare
jaceklaskowski
changed the title
|
Test build #75266 has finished for PR 17434 at commit
|
|
You'd asked I delivered @srowen |
srowen
left a comment
srowen
left a comment
There was a problem hiding this comment.
Some of these changes, I wouldn't make, not as 'typo' fixes. Most are good small fixes.
|
|
||
| /** | ||
| * Register a shuffle with the manager and obtain a handle for it to pass to tasks. | ||
| * Obtains a [[ShuffleHandle]] to pass to tasks. |
There was a problem hiding this comment.
Why remove some of the docs in instances like this? it's not obvious it was superfluous
There was a problem hiding this comment.
It's a copy from the main method this one overrides. Since the method does not do what the scaladoc said I thought I'd make it current. It might've "Register(ed) a shuffle with the manager" in the past but not today. It was misleading.
|
|
||
| /** | ||
| * Add an [[Aggregate]] to a logical plan. | ||
| * Add an [[Aggregate]] or [[GroupingSets]] to a logical plan. |
There was a problem hiding this comment.
Probably OK, but it's harder to know whether this is correct. I'm also aware that many [[xxx]] links actually fail to render in javadoc 8 -- have you tested the ones you added?
There was a problem hiding this comment.
Didn't check the javadoc and the change follows Aggregate. I'll see how to generate javadoc and check this out (as I've been meaning to do it anyway for some time). Thanks for inspiration!
There was a problem hiding this comment.
The package org.apache.spark.sql.catalyst.parser and hence AstBuilder don't show in javadoc/scaladoc.
| * | ||
| * dataFrame.createOrReplaceTempView("people") | ||
| * sparkSession.sql("select name from people").collect.foreach(println) | ||
| * sparkSession.sql("select name from people").show |
There was a problem hiding this comment.
I imagine this is an OK modification but it's not really a typo fix. I'd avoid changes that aren't fixing problems
There was a problem hiding this comment.
I agree. Just for the record, collect.foreach(println) should not be "endorsed" in Spark 2's official docs as too RDD-ish.
|
|
||
| /** | ||
| * Force spill during building. | ||
| * |
There was a problem hiding this comment.
It's not "for testing only". I'd even say that it's more often used in a non-test code than test code. That made the comment no longer correct. See ShuffleExternalSorter and UnsafeShuffleWriter which both are ShuffleWriter instances.
|
Test build #75301 has finished for PR 17434 at commit
|
|
Test build #75302 has finished for PR 17434 at commit
|
|
Closing as it was merged into #17417. |
3 participants
What changes were proposed in this pull request?
Just squashing a typo in
from_jsonfunctionHow was this patch tested?
local build