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

HTML5ify Javadoc for core and test framework #30234

Merged
merged 2 commits into from
Apr 30, 2018

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 29, 2018

javadoc will switch from detaulting to html4 to html5 in "a future
release". We should get ahead of it so we're not surprised. Also, HTML5
is the future! Er, the present. Anyway, this follows up from #30220 to
make the Javadoc for two of the four remaining projects HTML5
compatible.

`javadoc` will switch from detaulting to html4 to html5 in "a future
release". We should get ahead of it so we're not surprised. Also, HTML5
is the future! Er, the present. Anyway, this follows up from elastic#30220 to
make the Javadoc for two of the four remaining projects HTML5
compatible.
@@ -107,7 +107,7 @@ public static Boolean parseBoolean(String value, Boolean defaultValue) {
}

/**
* Returns <code>false</code> if text is in <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>; else, true
* Returns {@code false} if text is in "false", "0", "off", "no"; else, {@code true}.
Copy link
Member Author

Choose a reason for hiding this comment

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

<code> is still valid but {@code} is the "standard" way to do it in Javadoc and i was converting the <tt>s on the same line. I certainly wouldn't go out of my way to replace <code> with {@code} but this one was right there....

Copy link
Member

Choose a reason for hiding this comment

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

Personally I think that “null”, “true”, and “false” do not need to be wrapped in{@code } formatting. They occur so frequently it becomes a burden for the author with little if any benefit for the reader. This is not a comment to invoke change, only a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough. I like how it offsets the rendered text but when reading it un-rendered it doesn't add much.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -107,7 +107,7 @@ public static Boolean parseBoolean(String value, Boolean defaultValue) {
}

/**
* Returns <code>false</code> if text is in <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>; else, true
* Returns {@code false} if text is in "false", "0", "off", "no"; else, {@code true}.
Copy link
Member

Choose a reason for hiding this comment

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

Personally I think that “null”, “true”, and “false” do not need to be wrapped in{@code } formatting. They occur so frequently it becomes a burden for the author with little if any benefit for the reader. This is not a comment to invoke change, only a comment.

@@ -1703,7 +1703,7 @@ private int getNumClientNodes() {
}

/**
* This method is used to obtain settings for the <tt>Nth</tt> node in the cluster.
* This method is used to obtain settings for the {@code Nth} node in the cluster.
Copy link
Member

Choose a reason for hiding this comment

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

To be pedantic it should be Nth.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice! Done.

@@ -1978,7 +1978,7 @@ public Settings onNodeStopped(String nodeName) throws Exception {
}

/**
* Executed for each node before the <tt>n+1</tt> node is restarted. The given client is
* Executed for each node before the {@code n+1} node is restarted. The given client is
Copy link
Member

Choose a reason for hiding this comment

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

n + 1

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@nik9000 nik9000 merged commit 5094505 into elastic:master Apr 30, 2018
nik9000 added a commit that referenced this pull request Apr 30, 2018
`javadoc` will switch from detaulting to html4 to html5 in "a future
release". We should get ahead of it so we're not surprised. Also, HTML5
is the future! Er, the present. Anyway, this follows up from #30220 to
make the Javadoc for two of the four remaining projects HTML5
compatible.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 1, 2018
* origin/master:
  [test] add java packaging test project (elastic#30161)
  Fix macros in changelog (elastic#30269)
  [DOCS] Fixes syskeygen command name
  [ML] Include 3rd party C++ component notices (elastic#30132)
  _cluster/state Skip Test for pre-6.4, not pre-7.0 (elastic#30264)
  Improve docs for disk watermarks (elastic#30249)
  [DOCS] Removes redundant Active Directory realm settings (elastic#30190)
  [DOCS] Removes redundant LDAP realm settings (elastic#30193)
  _cluster/state should always return cluster_uuid (elastic#30143)
  HTML5ify Javadoc for core and test framework (elastic#30234)
  Minor tweaks to reroute documentation (elastic#30246)
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants