Jenkinsfile: disable javadoc, maven issues

[jukzi]

- publishAllIssues: false - to disable unrelated warnings in PRs

- maven: wrongly collects "API WARNING"s without resolving filename

- javadoc: it is currently not used

* eclipse: ecj info markers work fine with parsing xml files

[jukzi]

@laeubi @akurtakov @HannesWell @iloveeclipse
let's disable the not correctly working plugins on all repos, and be happy that at least ecj works now reliably (thanks @laeubi). If adding other plugins again please make sure they are working fine, collecting the right thinks.

[akurtakov]

I can see that Maven one is generating too much noise and is not actionable quite often . But why disable the javadoc one? IMHO it catches cases hat would otherwise fail the I-build and has no false positives, I know it's not catching all issues but it prevents a good chunk of them. The javadoc is used and all issues it catches are fixed - please try submitting a PR with javadoc issue to see.

[github-actions]

Test Results

   929 files   -     1     929 suites   - 1   42m 20s ⏱️ - 6m 28s
 7 474 tests ±    0   7 319 ✅  -     1  153 💤 ±  0  2 ❌ +2 
21 993 runs   - 1 578  21 601 ✅  - 1 458  390 💤  - 121  2 ❌ +2 

For more details on these failures, see this check.

Results for commit b945c88. ± Comparison against base commit e340eca.

[jukzi]

know it's not catching all issues but it prevents a good chunk of them..

i have not seen it catching any javadoc issue even though there are javadoc issues. until now i have only seen javadoc issues reported by ecj - handled by eclipse() plugin

[laeubi]

maven: wrongly collects "API WARNING"s without resolving filename

Why is this "wrong", it does not can show the warnings on the file but still it is to be fixed (I'm currently working on generating a ecj like xml file from API warnings here).

I can see that Maven one is generating too much noise and is not actionable quite often

PDE is down to one maven warning that I currently can't understand (it reports a difference in classfile that I can't reproduce locally) but beside that they are actionable....

i have not seen it catching any javadoc issue even though there are javadoc issues. until now i have only seen javadoc issues reported by ecj - handled by eclipse() plugin

ECJ does not handle javadoc quite well:

• Javadoc validation does not take into account implied end tags eclipse-jdt/eclipse.jdt.core#1858
• Javadoc issues eclipse-jdt/eclipse.jdt.core#1582

[laeubi]

publishAllIssues: false - to disable unrelated warnings in PRs

They are not unrelated they show where we have warnings that might be fixed as part of a PR.

[jukzi]

Why is this "wrong", it does not can show the warnings on the file but still it is to be fixed (I'm currently working on generating a ecj like xml file from API warnings here).

It gathers locations in the "console Output" instead of pointing to the offending .java file. As a result it randomly interprets those errors as "new" or "old" dependinig on the line number in the console output - which is almost random, since each commit can modify output lenght.
As soon you have a working solution we can enable that, but for now its just broken.

[jukzi]

They are not unrelated they show where we have warnings that might be fixed as part of a PR.

As soon as they fixed by the PR, then they are not shown. if you are interested in Warnings that could be fixed you can simply go to jenkins or problems view to see all existing ones. But there is no need to pollute every PR with that information.m Nobody reads that and even less people actually fix them.

[laeubi]

It gathers locations in the "console Output" instead of pointing to the offending .java file. As a result it randomly interprets those errors as "new" or "old" dependinig on the line number in the console output - which is almost random, since each commit can modify output lenght.

That's wrong, these warnings are always there, they are just not visible most of the time as the API tools are skipped when the baseline replace kicks in (for performance reasons).

As soon you have a working solution we can enable that, but for now its just broken.

It is the best we can offer with a home brew API tools solution that no one else in the industry seem to use...

But there is no need to pollute every PR with that information.m Nobody reads that and even less people actually fix them.

If nobody is reading them there can't be an issue in showing them...

[laeubi]

Disabling warnings just because they annoy people is a no-go for me. Without showing problems they won't get fixed.

[jukzi]

Disabling warnings just because they annoy people is a no-go for me. Without showing problems they won't get fixed.

they are not disabled, they still visible in the console log. Only that PRs are not polluted with unrelated or false positives.

If nobody is reading them there can't be an issue in showing them...

They are visible and distract from real related issues. They are annoying even if you don't read them. like popups/disclaimer you never read but accept blindly.

[iloveeclipse]

Only that PRs are not polluted with unrelated or false positives.

I've expressed many times my opinion on this, and I agree that one should not enable additional checks on PR's as long as the checked issues are not fixed and so all PR's marked as "failed" because of the (unrelated) checks.

I appreciate work on fixing warnings, but this work should not interfere with regular work and should not confuse contributors.

A PR that makes a change that builds & run tests without introducing new warnings or errors should be always green.

If it is not, and additionally because of unrelated code, it only confuses everyone. Also people learn to ignore all kind of PR validation errors because "they are always there".

let's disable the not correctly working plugins on all repos

Could you please link some examples of recent PR's with false positives?

[jukzi]

Could you please link some examples of recent PR's with false positives?

Currently all PRs are affected to show the issues stated in #1557 (comment). That does not mean they are "red", but the details shown in the jenkins/ github PR are disturbing - or even wrong.

[akurtakov]

@jukzi

from https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/PR-1558/2/ shows a warning catched by javadoc plugin that is not catched by ecj plugin. I hope this is good enough proof.

[jukzi]

it finds a warning and can not relate it to any file in the repository .. i hope that is proof enough that it is not helpfull

12:58:17.281 [INFO] --- javadoc:3.6.3:jar (attach-javadocs) @ org.eclipse.jface ---
12:58:17.668 [INFO] No previous run data found, generating javadoc.
12:58:27.922 [WARNING] Javadoc Warnings
12:58:27.922 [WARNING] /home/jenkins/agent/workspace/eclipse.platform.ui_PR-1558/bundles/org.eclipse.jface/src/org/eclipse/jface/action/Action.java:28: warning: empty <p> tag
12:58:27.922 [WARNING] * <p>
12:58:27.922 [WARNING] ^
...
[JavaDoc] [-ERROR-] Can't create fingerprints for some files:
[JavaDoc] [-ERROR-] - '12:58:27.922 [WARNING] /home/jenkins/agent/workspace/eclipse.platform.ui_PR-1558/bundles/org.eclipse.jface/src/org/eclipse/jface/action/Action.java' file not found
[JavaDoc] [-ERROR-] Can't create fingerprints for some files:
[JavaDoc] [-ERROR-] - '12:58:27.922 [WARNING] /home/jenkins/agent/workspace/eclipse.platform.ui_PR-1558/bundles/org.eclipse.jface/src/org/eclipse/jface/action/Action.java' file not found

=> warning not shown on github:

[laeubi]

it finds a warning and can not relate it to any file in the repository ..

Sometimes I really get the impression people "don't see it" because they don't want to see it:

=> warning not shown on github:

[jukzi]

the numbers are correct, but the warning it is supposed to be shown in the review tab like every ecj marker. somehow it is not configured right. i suggest you get it fully working before enabling it

[akurtakov]

@jukzi So showing a warning (no false positives found so far) with a direct link to the code line is considered not helpful because not rendered in Github UI .
I simply give up here.

[laeubi]

the numbers are correct, but the warning it is supposed to be shown in the review tab like every ecj marker. somehow it is not configured right. i suggest you get it fully working before enabling it

Sorry to say but that's nonsense, code annotations are a completely different topic, so your claim it does not find issues is simply proven wrong so please stop to invent new argument of "not working".

[jukzi]

I simply give up here.

And that's the problem with this tools: only half configured it would stay that way until the end. Instead you could try to fix that error. May be it would be enough to convert absolute path to a relative one.

[laeubi]

May be it would be enough to convert absolute path to a relative one.

I already told you several times, if you suspect a bug or improvement the please report it here: https://github.com/jenkinsci/warnings-ng-plugin "we" can't do anything here (only if the bug is fixed to ask infrateam to update Jenkins).

[jukzi]

Thats exactly the mood i don't like: installing something broken and leave the cleanup for others.

[laeubi]

Thats exactly the mood i don't like: installing something broken and leave the cleanup for others.

The difference is just the interpretation of "broken" ... for me it is nothing broken and I can perfectly work with it (and it seems you even used that in the past PRs) now it is suddenly "broken" for you ...

So for me the problem is more searching "fly in the ointment" than actually aiming for something to improve.

[jukzi]

I have not used that tools. in opposite: i am steady confused by them - for example javadoc telling me there are no issues while there are. For javadoc the one and only way is to grep the console log manually.
Or maven constantly telling me new issues introduced while nothing changed.

[akurtakov]

This is not leading anywhere.
As per https://github.com/eclipse-platform/.github/wiki/PMC-project-guidelines#committer-disagreement-resolution I step up as a PL and ask you @jukzi to ask for vote on the topic on platform-dev. That would help us understand how majority of committers feel about the tooling and its usefulness.
I deliberately skip steps from the process and ask for the vote directly for the already explained reason. So far I see 3 options:

• disable maven (some non actionable/resolvable warnings) and javadoc (no such but GH/Jenkins UI not rendering annotations)
• disable only maven(due to non actionable/resolvable warnings)
• disable none (report all warnings even as they warn about possible problems)

[akurtakov]

I actually took the liberty to experiment with GH polls #1567 . Everyone, please feel free to fix the wording on the poll, I tried to be as objective as I could in the options.

[laeubi]

javadoc telling me there are no issues while there are. For javadoc the one and only way is to grep the console log manually.

Please share an example, on the other hand why are you not "steady confused" and we should not disable ECJ warnings then as it tells there are not javadoc issues while there are some?

A PR that makes a change that builds & run tests without introducing new warnings or errors should be always green.
If it is not, and additionally because of unrelated code, it only confuses everyone.

...

Or maven constantly telling me new issues introduced while nothing changed.

I have opened

• Don't skip api analysis if artifact was replaced to always get messages eclipse.platform.releng.aggregator#1747

[laeubi]

@vogella just as we have talked about here (but I don't want to spam the thread more so I think its fine to continue here!)

I mentioned here a problem with the display of the status:

• [JENKINS-72059] Add new quality gate options to alter the stage only jenkinsci/warnings-ng-plugin#1649 (comment)

The user gracefully opened a PR here:

• Fix naming of unstable status jenkinsci/github-branch-source-plugin#757

What one can see as of today:

Really amazing!

[laeubi]

By the way API issues will soon no longer be reported as Maven warnings / errors but listed in the ECJ issues:

• Redirect API messages to the ECJ log eclipse.platform.releng.aggregator#1751

Also shown as code annotations here:

And as I assume it is "super confusing" I'll propose to rename the check from "Eclipse Compiler" to "Eclipse Compiler and API problem" soon, please let me know if you prefer a different naming...