Page MenuHomePhabricator

Phan CI jobs should stop using checkstyle but use a text report instead
Closed, ResolvedPublic

Description

The Phan jobs emit a checkstyle report which is barely readable for humans. We could use the Jenkins publisher report but it is not that helpful since:

  • we point users to the console log
  • it shows a diff compared to the previous build which is an entirely unrelated commit / repository.

Seems nicer to just print a text report (the default) with colors.

Event Timeline

hashar created this task.Aug 7 2020, 2:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 7 2020, 2:26 PM
Daimona added a subscriber: Daimona.Aug 7 2020, 3:15 PM

I think the general direction we and the larger industry/community have been moving away from dedicated GUI viewers.

I think there's two reasons for that:

  1. They tend to be optimised for classical post-merge jobs. E.g. the PHPUnit viewer in Jenkikns offers "trends" based on previous jobs, but that is meaningless in a pre-merge context since it compares to unrelated jobs. Anything wrong or above thresholds is found and reported directly on each commit. No need to compare. We have absoloute thresholds, and don't tolerate build failures in master anyway.
  2. They are not portable. The Jenkins viewer isn't applicable to local development. It also isn't applicable to e.g. Travis CI etc.

In general, it seems much focus has moved towards making better and more portable console reporters. For QUnit, PHPUnit, ESLint etc we have good CLI reporters that are high signal, low noise, and have everything we need.

For PHPCS, Phan and Karma we currently have very unusable CLI output. Are there better formatters we can configure for those? That seems like a potentially more worthwile investment. E.g. we don't need to output checkstyle XML from Phan, it has a "plaintext" output format that uses colorized pretty print text optimised for consoles and CI build output.

Thank you for the input @Krinkle Indeed, looks like for phan we have some nice output with phan --color --output-mode text.

hashar renamed this task from Phan CI jobs should emit a checkstyle XML file to Phan CI jobs should stop using checkstyle but use a text report instead.Fri, Sep 25, 7:55 AM
hashar updated the task description. (Show Details)
hashar added a parent task: Restricted Task.

Change 630068 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Change Phan jobs to output text instead of checkstyle

https://gerrit.wikimedia.org/r/630068

Mentioned in SAL (#wikimedia-releng) [2020-09-25T08:59:49Z] <hashar> Updating phan jobs to use text output instead of checkstyle. https://gerrit.wikimedia.org/r/c/integration/config/+/630068/ | T259890

Ammarpad added a subscriber: Ammarpad.EditedFri, Sep 25, 11:43 AM

Some of the colors are really not good on white background.

Indeed it is cringy. There a few other color schemes available which can be changed with --color-scheme or setting PHAN_COLOR_SCHEME environment variable.

vim

The vim one has some black against red which is barely readable.

light

After discussions with @Daimona , it seems that the light theme gives the best outcome so far. Even though it is far from perfect.

Mentioned in SAL (#wikimedia-releng) [2020-09-25T12:03:26Z] <hashar> Updated phan jobs to use PHAN_COLOR_SCHEME=light # T259890

Ammarpad added a comment.EditedFri, Sep 25, 12:05 PM

Glad to know the issue is already known and being discussed. Yes, either of these two themes may apparently be better than the one in T259890#6493908.

Change 630068 merged by jenkins-bot:
[integration/config@master] Change Phan jobs to output text instead of checkstyle

https://gerrit.wikimedia.org/r/630068

hashar closed this task as Resolved.Fri, Sep 25, 12:57 PM

We now use a text report instead of checkstyle.

When using the white background, a better color schme should be used which is filed upstream as https://github.com/phan/phan/issues/4203