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

Update custom Doclet configuration for javadoc #587

Merged
merged 5 commits into from
Sep 22, 2021
Merged

Conversation

lahirumaramba
Copy link
Member

  • Starting with version 3.0 javadoc uses updated parameter names in custom Doclet configurations, which affects our internal devsite scripts.
  • This PR updates the config to use the new parameters names.
  • Tested and verified with the devsite script.

From version 3.0 and up `javadoc` has updated parameter names used in custom Doclet configurations, which affects our internal devsite scripts.
This PR updates the config to use the new parameters names.
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. Just one suggestion.

pom.xml Show resolved Hide resolved
@lahirumaramba
Copy link
Member Author

Looks like line 87 still uses the current config name. Updated the rest. Thanks!

@hiranya911
Copy link
Contributor

Thanks. May be we can remove that profile entirely, and just merge the configuration with the rest? It's reasonable to require jdk 1.8 or higher for all build and release tasks:

       <profile>
            <!-- Disable Doclint on Java 8 and higher -->
            <id>disable-java8-doclint</id>
            <activation>
                <jdk>[1.8,)</jdk>
            </activation>
            <properties>
                <additionalparam>-Xdoclint:none</additionalparam>
            </properties>
        </profile>

@lahirumaramba
Copy link
Member Author

Thanks. May be we can remove that profile entirely, and just merge the configuration with the rest? It's reasonable to require jdk 1.8 or higher for all build and release tasks:

       <profile>
            <!-- Disable Doclint on Java 8 and higher -->
            <id>disable-java8-doclint</id>
            <activation>
                <jdk>[1.8,)</jdk>
            </activation>
            <properties>
                <additionalparam>-Xdoclint:none</additionalparam>
            </properties>
        </profile>

Updated and dropped this profile.

-warning 101
</additionalparam>
<additionalOptions>
<additionalOption>-hdf book.path /docs/reference/_book.yaml</additionalOption>
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to pass the -Xdoclint:none option here too. Earlier it was provided by the profile whenever JDK 1.8 was available. Now we have to explicitly set it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Added <doclint>none</doclint>

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion.

</additionalparam>
<additionalOptions>
<additionalOption>-warning 101</additionalOption>
</additionalOptions>
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably need doclint:none here too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Thanks!

@lahirumaramba lahirumaramba merged commit 89ac0d0 into master Sep 22, 2021
@lahirumaramba lahirumaramba deleted the lm-javadoc-fix branch September 22, 2021 21:27
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.

2 participants