-
Notifications
You must be signed in to change notification settings - Fork 471
Improve generated server properties documentation #5936
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
base: main
Are you sure you want to change the base?
Conversation
92126c6 to
0167507
Compare
* Use gson to pretty-print default values for JSON property types * Use html tables instead of markdown to support multi-line descriptions and default values * Support markdown syntax in property descriptions by enabling kramdown support for mixing markdown and html table cells * Stop replacing newlines with `<br>` in property descriptions, and just accept the property description written in markdown * Fix documentation for restarting the manager when a compaction.coordinator property changes * Improve PropertyTest and PropertyTypeTest to ensure property descriptions and default values restrictions are strictly enforced * Update Property descriptions to use markdown, and for improved readability * Add more links to other properties and classes in the Property descriptions * Wrap some Property descriptions better, and remove trailing backslashes, since they are no longer needed for markdown paragraphs * Where examples are used for setting properties, show in a style that can be used directly in the properties config file, with proper escaping * Standardize using `0` when a zero-value has special meaning for a property, so that the value described is shown the same way a user would set it (i.e. say `0` instead of "zero", to make it clear that the configuration would look like a literal `0`) * Use class.getName() where possible for class references, and fix several classes whose class name was incorrectly hard-coded * Fix several properties to use PropertyType.CLASSNAME instead of PropertyType.STRING when their values are a class name type * Fix generated document to use the short name of the property type, rather than the enum name, which is only in code * Remove old information about CDATA, since configuration is no longer XML-based * Remove reference to deprecated properties, and update docs to specify replacements * Update PropertyType descriptions to use markdown also This fixes apache#5902 and apache#5903
0167507 to
7692cac
Compare
DomGarguilo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a look through the rendered page differences and everything looks good. For context/reference here is what I saw:
Current view:

New view:

Things are a bit more spread out but the table is definitely formatted better.
Current view:

New view:

This shows the improved code formatting and line wrapping.
|
@ctubbsii was there anything you wanted to do before merging this? I have not looked at the changes, was just looking through PRs and saw it was approved. |
I have a local branch I need to reconcile and see if this includes everything. Will try to do that tomorrow, but prioritizing looking at the accumulo-classloader PR first |
descriptions and default values
support for mixing markdown and html table cells
<br>in property descriptions, and justaccept the property description written in markdown
compaction.coordinator property changes
descriptions and default values restrictions are strictly enforced
readability
descriptions
backslashes, since they are no longer needed for markdown paragraphs
can be used directly in the properties config file, with proper
escaping
0when a zero-value has special meaning for aproperty, so that the value described is shown the same way a user
would set it (i.e. say
0instead of "zero", to make it clear thatthe configuration would look like a literal
0)several classes whose class name was incorrectly hard-coded
PropertyType.STRING when their values are a class name type
rather than the enum name, which is only in code
XML-based
replacements
This fixes #5902 and #5903