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

chore: Improve ci-checkstyle-javadoc.sh usability #3924

Merged

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented May 12, 2021

Fix #3922

This PR improves the usability of the method Javadoc quality script, and links to the issue related to using it #3923. Additionally, non-POSIX-compliant comparisons have been removed to make the script compatible with more shells.

In terms of output, if the script failed in CI it would previously just print the amount of errors, but now ALSO prints this:

Run the chore/ci-checkstyle-javadoc.sh script locally to find errors
See https://github.com/inria/spoon/issues/3923 for details

Local use has also been improved, here's the help output:

$ ./chore/ci-checkstyle-javadoc.sh
usage: ci-checkstyle-javadoc.sh [<regex>|COMPARE_WITH_MASTER]

    <regex>: A regex to filter output lines by (typically just a file path)
    COMPARE_WITH_MASTER: Compare the amount of errors on the current
        branch with those on master and exit non-zero if the current branch has
        more errors. WARNING: Never run this with uncommitted changes, they
        will be lost!

See https://github.com/inria/spoon/issues/3923 for info related to using this script locally to improve Javadoc quality.

To list all errors in a particular .java file, just run with '/<CLASS_NAME>.java' as the argument. For example:

    $ ./chore/ci-checkstyle-javadoc.sh '/Launcher.java'

And, an example execution:

./chore/ci-checkstyle-javadoc.sh '/Launcher.java'
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:103:42: Expected @param tag for 'args'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:180:52: Expected @param tag for 'resource'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:604: Expected @return tag. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:644: Expected @return tag. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:644:57: Expected @param tag for 'factory'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:644:86: Expected @param tag for 'inputSources'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:644:120: Expected @param tag for 'templateSources'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:660: Expected @return tag. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:660:69: Expected @param tag for 'inputSources'. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:873: Expected @return tag. [JavadocMethod]
[ERROR] /home/slarse/Documents/github/cdate/spoon-slarse/src/main/java/spoon/Launcher.java:873:52: Expected @param tag for 'code'. [JavadocMethod]

All-in-all, this makes the script actually useful for improving the docs!

Comment on lines -69 to +76
elif [[ $compare_num_errors < $current_num_errors ]]; then
elif [ $compare_num_errors -lt $current_num_errors ]; then
Copy link
Collaborator Author

@slarse slarse May 17, 2021

Choose a reason for hiding this comment

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

Technically, this is a bugfix masquerading as a compatibility improvement, as < in extended expression syntax actually does a lexicographical comparison. That is to say, something like [[ 10000 < 2 ]] evaluates to true, whereas -lt actually does a numeric less-than comparison (and is equivalent to (( 1000 < 2 )), note round brackets). Oopsie!

@slarse
Copy link
Collaborator Author

slarse commented May 17, 2021

This is curious, the PR failed a unit test, even though it doesn't do anything with the test suite or production code: https://github.com/INRIA/spoon/pull/3924/checks?check_run_id=2598716911#step:10:480

Error:  Failures: 
Error:    MavenLauncherTest.multiModulesProjectTest:127 expected:<166> but was:<118>

Flaky?

@slarse slarse closed this May 17, 2021
@slarse slarse reopened this May 17, 2021
@slarse
Copy link
Collaborator Author

slarse commented May 17, 2021

Yep, flaky, closing and re-opening caused the workflow to pass.

@slarse slarse changed the title wip: chore: Improve ci-checkstyle-javadoc.sh usability review: chore: Improve ci-checkstyle-javadoc.sh usability May 17, 2021
@monperrus
Copy link
Collaborator

LGTM, will merge.

@monperrus monperrus changed the title review: chore: Improve ci-checkstyle-javadoc.sh usability chore: Improve ci-checkstyle-javadoc.sh usability May 18, 2021
@monperrus monperrus merged commit ea40e39 into INRIA:master May 18, 2021
@slarse slarse deleted the issue/3922-improve-checkstyle-javadoc-usability branch May 18, 2021 10:01
@monperrus monperrus mentioned this pull request Aug 19, 2021
woutersmeenk pushed a commit to woutersmeenk/spoon that referenced this pull request Aug 29, 2021
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.

Improve usability of the ci-checkstyle-javadoc.sh script for local use
2 participants