[perceptron8]

This commit reverts the behavior introduced in d3fbbda, after which a warning about the -1.1 switch appears unconditionally. The warning, "Javadoc 1.4+ doesn't support the -1.1 switch anymore. Ignore this option." now only displays if the old parameter is set to true, as it did previously.

---

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

[olamy]

can you please provide an IT test for this as I'm not really sure to understand the problem.

[perceptron8]

Sure! Here it is: it.zip.

Compare

mvn clean org.apache.maven.plugins:maven-javadoc-plugin:3.11.2:jar

with

mvn clean org.apache.maven.plugins:maven-javadoc-plugin:3.11.3:jar

In case of 3.11.3 you will get:

[WARNING] Javadoc 1.4+ doesn't support the -1.1 switch anymore. Ignore this option.

The warning shouldn't be displayed when old is false (which is the default setting).

[olamy]

Thanks.
I have modifiied an IT test for your change in this PR.

[perceptron8]

Could we add another test? This one would check if assert log.count(...) == 1 passes for old = true. The existing test will pass not only when there is no warning, but also if the warning is displayed with slightly different wording. If the message is altered in the future, this test won't be a reliable check.

[olamy]

Could we add another test? This one would check if assert log.count(...) == 1 passes for old = true. The existing test will pass not only when there is no warning, but also if the warning is displayed with slightly different wording. If the message is altered in the future, this test won't be a reliable check.

sure feel free to add more tests and let me know when you're done then I will merge the PR

[perceptron8]

It seems that this would require a completely new integration test (with a separate pom.xml, invoker.properties, etc., for example, in MJAVADOC-770_old). It appears that MJAVADOC-770* is not a good place at all to test whether the old-related warning appears (this IT seems to be testing the legacyMode parameter instead). Therefore, I wanted to propose removing the assertion you added. In its current form, it's not very useful (for the reason I presented in my previous comment). Adding another test with positive case just for checking a minor warning is probably an overkill.

[olamy]

It seems that this would require a completely new integration test (with a separate pom.xml, invoker.properties, etc., for example, in MJAVADOC-770_old). It appears that MJAVADOC-770* is not a good place at all to test whether the old-related warning appears (this IT seems to be testing the legacyMode parameter instead). Therefore, I wanted to propose removing the assertion you added. In its current form, it's not very useful (for the reason I presented in my previous comment). Adding another test with positive case just for checking a minor warning is probably an overkill.

sure go for it. maybe just create a new IT as you can see it's very easy :)

[perceptron8]

Ok, I'll clean up MJAVADOC-770 and add 2 old-related ITs.

[perceptron8]

Please do squash the first 4 commits when merging (hopefully). I didn't want to force-push to avoid disrupting your review of the PR.

[olamy]

@perceptron8 Thanks. I'm waiting for all CI builds to pass then will merge it.