Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#SPARK-16911] Fix the links in the programming guide #14503

Closed
wants to merge 4 commits into from

Conversation

shiv4nsh
Copy link
Contributor

@shiv4nsh shiv4nsh commented Aug 5, 2016

What changes were proposed in this pull request?

Fix the broken links in the programming guide of the Graphx Migration and understanding closures

How was this patch tested?

By running the test cases and checking the links.

@@ -1097,7 +1097,7 @@ for details.
<tr>
<td> <b>foreach</b>(<i>func</i>) </td>
<td> Run a function <i>func</i> on each element of the dataset. This is usually done for side effects such as updating an <a href="#accumulators">Accumulator</a> or interacting with external storage systems.
<br /><b>Note</b>: modifying variables other than Accumulators outside of the <code>foreach()</code> may result in undefined behavior. See <a href="#ClosuresLink">Understanding closures </a> for more details.</td>
<br /><b>Note</b>: modifying variables other than Accumulators outside of the <code>foreach()</code> may result in undefined behavior. See <a href="#understanding-closures-a-nameclosureslinka">Understanding closures </a> for more details.</td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These anchors don't exist, are you sure about this? or close this PR please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srowen : When we see on the documentation: https://spark.apache.org/docs/latest/programming-guide.html#understanding-closures-a-nameclosureslinka Points to the understanding Closure documentation part hence I replaced it here. I have also rechecked it by generating the docs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right this is an auto-generated anchor. this is OK.

@shiv4nsh
Copy link
Contributor Author

shiv4nsh commented Aug 5, 2016

@srowen : So can we merge this ? or do I have to make some more changes?

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Did you see my last comment? I think we should remove the old migration guides in latest Spark 2 docs.

@shiv4nsh
Copy link
Contributor Author

shiv4nsh commented Aug 5, 2016

@srowen : So do we have to remove this whole part?
screenshot from 2016-08-05 13-04-47

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Personally, I think that any documentation that talks about migrating to a Spark 1.x version can be removed from the 2.x documentation in master. The target of such a migration is also obsolete. You could make this JIRA really about that, as well as fixing the first link as a side effect.

@shiv4nsh
Copy link
Contributor Author

shiv4nsh commented Aug 5, 2016

@srowen : I have updated the documentation and here is what it looks like :
screenshot from 2016-08-05 14-21-36

Please review it ! Thanks :)

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Yes, what about removing the streaming/graphx migration guides? they're also about very old versions.

@shiv4nsh
Copy link
Contributor Author

shiv4nsh commented Aug 5, 2016

@srowen : I have removed them too and now it looks like this:

Graphx Guide :

graphx

Streaming Guide:

spark_streaming

Please review them ! Thanks :)

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Aug 5, 2016

Test build #63266 has finished for PR 14503 at commit c5c6c34.

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

@shiv4nsh
Copy link
Contributor Author

shiv4nsh commented Aug 5, 2016

@srowen : Can we merge this now ?

@srowen
Copy link
Member

srowen commented Aug 5, 2016

Yes, though we conventionally leave things open for review for a time. You don't need to ping so quickly.

asfgit pushed a commit that referenced this pull request Aug 7, 2016
## What changes were proposed in this pull request?

 Fix the broken links in the programming guide of the Graphx Migration and understanding closures

## How was this patch tested?

By running the test cases  and checking the links.

Author: Shivansh <shiv4nsh@gmail.com>

Closes #14503 from shiv4nsh/SPARK-16911.

(cherry picked from commit 6c1ecb1)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@srowen
Copy link
Member

srowen commented Aug 7, 2016

Merged to master/2.0

@asfgit asfgit closed this in 6c1ecb1 Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants