Skip to content

Conversation

@hvanhovell
Copy link
Contributor

Tiny modification to a few comments sbt publishLocal work again.

@yhuai
Copy link
Contributor

yhuai commented Aug 14, 2015

test this please

Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't more valid as HTML. Was this just an extraneous change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it turns out that javadoc in java 8 doesn't allow self-closing elements (<br/> and <p/>) any more:
http://stackoverflow.com/questions/26049329/javadoc-in-jdk-8-invalid-self-closing-element-not-allowed
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#format

<p> is the preferred seperator for a paragraph. So its is not HTML but Javadoc we are talking about. Sorry about the confusion.

Copy link
Member

Choose a reason for hiding this comment

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

Hm I though that's because it now required valid HTML, like <p>...</p>. Well, if it fixes an error, obviously that's better than an error. There seem to be ~17 occurrences of this in the code base though -- worth fixing it in one go?

@srowen
Copy link
Member

srowen commented Aug 14, 2015

I found several instances of a similar javadoc problem in:

  • BytesToBytesMap
  • Bin
  • PagedTable

Let's clean those up along the way?

@hvanhovell
Copy link
Contributor Author

I'll change those as well. It is strange that these didn't create problems; I guess it has something to do with the position of the << in the line.

@hvanhovell
Copy link
Contributor Author

I have replaced <p/> tags by <p> in all java files I could find them in. I haven't touched the Bin and PagedTable classes because these are written scala and Scaladoc has no problem with <<'s.

@srowen
Copy link
Member

srowen commented Aug 14, 2015

Hm, still seems better for consistency and/or to future-proof; to the extent the docs are read as HTML it isn't valid. I don't feel super strongly about addressing it now though if it's not causing an error.

@hvanhovell
Copy link
Contributor Author

I'd rather leave the scala source alone for now.

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40896 has finished for PR 8209 at commit 4a743c2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #1607 has finished for PR 8209 at commit 9111dc0.

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

asfgit pushed a commit that referenced this pull request Aug 15, 2015
…ters in doc

Tiny modification to a few comments ```sbt publishLocal``` work again.

Author: Herman van Hovell <[email protected]>

Closes #8209 from hvanhovell/SPARK-9980.

(cherry picked from commit a85fb6c)
Signed-off-by: Sean Owen <[email protected]>
@asfgit asfgit closed this in a85fb6c Aug 15, 2015
CodingCat pushed a commit to CodingCat/spark that referenced this pull request Aug 17, 2015
…ters in doc

Tiny modification to a few comments ```sbt publishLocal``` work again.

Author: Herman van Hovell <[email protected]>

Closes apache#8209 from hvanhovell/SPARK-9980.
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.

4 participants