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-18198][Doc][Streaming] Highlight code snippets #15715

Closed
wants to merge 2 commits into from

Conversation

lw-lin
Copy link
Contributor

@lw-lin lw-lin commented Nov 1, 2016

What changes were proposed in this pull request?

This patch uses {% highlight lang %}...{% endhighlight %} to highlight code snippets in the Structured Streaming Kafka010 integration doc and the Spark Streaming Kafka010 integration doc.

This patch consists of two commits:

  • the first commit fixes only the leading spaces -- this is large
  • the second commit adds the highlight instructions -- this is much simpler and easier to review

How was this patch tested?

SKIP_API=1 jekyll build

Screenshots

Before

snip20161101_3

After

snip20161101_1

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

Fix the leading spaces is needed, because without the fixing the code snippet would contain leading spaces as well(see the pic below), which is quite inconsistent with all other programming guides.

snip20161101_5

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

@koeninger @srowen it'd be great if you could also take a look at this too :)

Hint: this patch consists of two commits:

  • the first commit fixes only the leading spaces -- this is large
  • the second commit adds the highlight instructions -- this is much simpler and easier to review

@srowen
Copy link
Member

srowen commented Nov 1, 2016

That is nicer. Are there any other code snippets across the code base that need this treatment? seems like this is always the right thing to do.

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

@srowen thanks.

I'm afraid the streaming flume integration doc, kinesis integration doc, as well as kafka08 integration doc also need code highlights, but this {% hightlight %} syntax seem to be conflict with the indentation syntax (e.g. https://github.com/apache/spark/blame/branch-2.0/docs/streaming-flume-integration.md#L40-L73). I guess that's why the flume, kinesis, kafka08 docs don't have code highlights in the first place. structured streaming kafka010, spark streaming kafka010doc, on the other hand, don't have such conflicts -- so we're adding code highlights here.

And as far as I can tell, no other code snippets need this treatment.

@SparkQA
Copy link

SparkQA commented Nov 1, 2016

Test build #67899 has finished for PR 15715 at commit 5f2c21b.

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

@srowen
Copy link
Member

srowen commented Nov 1, 2016

What about unindenting those blocks? I don't see why they have to be in the first place. They're HTML tags.

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

snip20161101_7

Please see the screenshot above -- this is flume doc -- I got this by unindenting the second block. Then:

  • the second block's indent does not look as pretty as the first's and the third's
  • the third block's numbering just goes wrong -- it should have been 3.

@srowen
Copy link
Member

srowen commented Nov 1, 2016

What about keeping the indent of the div tags but un-indenting only the code? it might look weird in markdown but does it look nice in the rendered output?

If it's any more trouble than that, forget it.

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

Good question!

Actually I had tried keeping the indent of the div tags but un-indenting only the code before I open this pr -- and it looked like this:

snip20161102_8

That was when I gave up trying more on the flume, kinesis, kafka08 docs. :-D

@srowen
Copy link
Member

srowen commented Nov 1, 2016

I guess we could restructure the headings to not have these numbered, indented sections to begin with, but that's a separate task I think.

@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 1, 2016

Sounds good!

@koeninger
Copy link
Contributor

LGTM

@HyukjinKwon
Copy link
Member

+1

@srowen
Copy link
Member

srowen commented Nov 2, 2016

Merged to master

@asfgit asfgit closed this in 98ede49 Nov 2, 2016
@lw-lin
Copy link
Contributor Author

lw-lin commented Nov 2, 2016

@srowen thanks -- could we also merge this into branch-2.1?

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

This patch uses `{% highlight lang %}...{% endhighlight %}` to highlight code snippets in the `Structured Streaming Kafka010 integration doc` and the `Spark Streaming Kafka010 integration doc`.

This patch consists of two commits:
- the first commit fixes only the leading spaces -- this is large
- the second commit adds the highlight instructions -- this is much simpler and easier to review

## How was this patch tested?

SKIP_API=1 jekyll build

## Screenshots

**Before**

![snip20161101_3](https://cloud.githubusercontent.com/assets/15843379/19894258/47746524-a087-11e6-9a2a-7bff2d428d44.png)

**After**

![snip20161101_1](https://cloud.githubusercontent.com/assets/15843379/19894324/8bebcd1e-a087-11e6-835b-88c4d2979cfa.png)

Author: Liwei Lin <[email protected]>

Closes #15715 from lw-lin/doc-highlight-code-snippet.

(cherry picked from commit 98ede49)
Signed-off-by: Sean Owen <[email protected]>
@srowen
Copy link
Member

srowen commented Nov 2, 2016

Oops, I see, we've cut branch 2.1. I must have missed that one too along with other dev@ lists going to spam. Yes I will go back and make sure a few I've just merged to master are in 2.1.

@lw-lin lw-lin deleted the doc-highlight-code-snippet branch November 2, 2016 13:07
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

This patch uses `{% highlight lang %}...{% endhighlight %}` to highlight code snippets in the `Structured Streaming Kafka010 integration doc` and the `Spark Streaming Kafka010 integration doc`.

This patch consists of two commits:
- the first commit fixes only the leading spaces -- this is large
- the second commit adds the highlight instructions -- this is much simpler and easier to review

## How was this patch tested?

SKIP_API=1 jekyll build

## Screenshots

**Before**

![snip20161101_3](https://cloud.githubusercontent.com/assets/15843379/19894258/47746524-a087-11e6-9a2a-7bff2d428d44.png)

**After**

![snip20161101_1](https://cloud.githubusercontent.com/assets/15843379/19894324/8bebcd1e-a087-11e6-835b-88c4d2979cfa.png)

Author: Liwei Lin <[email protected]>

Closes apache#15715 from lw-lin/doc-highlight-code-snippet.
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.

5 participants