Psalm baseline feature has a significant downside that it could allow reported issues to be silenced without proper review or consideration.
Extra care is required to review baseline diff and while added entries might look innocuous enough they might be pointing at a real problem visible in context.
Proper review currently requires checking component locally, resetting the baseline changes and then running the psalm. Occasionally due to runtime PHP version differences psalm will report differently which further complicates review.
I suggest we introduce two new additional psalm jobs.
Job to report newly baselined psalm errors in the current PR
No failure psalm job that would checkout targeted branch baseline to run against with the exception of explicitly ignoring unused baseline entries.
This job would be an important review tool that would allow to see exactly which problems were baselined by a given PR without having to guess from baseline diff or having to checkout code and run psalm locally.
I think this job should report found errors as check annotations on the PR as well, although with reduced severity - notice instead of error.
Check annotations with the current beta feature are shown in context even for files that were not directly changed by the PR. This allows to view error in context without having to navigate to the code in a separate tab.
To distinguish real psalm errors from baselined errors this job should rewrite github formatted psalm output to report as Notice instead of Warning or Error. Custom output formatter would be right way to do it but simple sed will work just as fine, until and unless github changes format.
See GHA docs: Setting a notice message and Psalm GithubActionsReport formatter
In slack discussion @Ocramius expressed skepticism about usefulness of annotations on pull request Files Changed tab over the visual noise they create. I believe annotations should be utilized for a couple reasons:
They would make significantly easier to review and approve or reject baselined issues especially because they are shown in context. Reviewers would be immediately alerted of any possible issues without having to dive into green action logs or extra carefully considering baseline diff.
Psalm errors reported as annotation events when baselined, although at lesser severity, would send a strong indication for contributors that they can not just sweep any issue under the rug with the baseline.
Annotation would make easier to refer in submitted review, to tell which issue must be actually fixed for the PR to succeed. As opposed to having to link to actions log. They would better facilitate dialog and give better chance to challenge and defend decisions to add to baseline.
Minor benefit in annotations from last action run remaining in PR long after the action log is gone, which is 90 days currently iirc.
Job to report all psalm baselined errors in action log
No failure --ignore-baseline run with default output formater that would not show in PR as failure or as PR annotations.
This job would allow to see in action log exactly which issues are currently baselined.
Bonus points for reporting total number of errors found as check notice message to be displayed as annotation on PR with something like 152 baselined psalm errors found. The way github shows annotations with "View workflow job for this annotation" button would serve as a way to increase awareness for contributors and link directly at reported baselined errors without getting in the way.
Feature Request
Summary
Psalm baseline feature has a significant downside that it could allow reported issues to be silenced without proper review or consideration. Extra care is required to review baseline diff and while added entries might look innocuous enough they might be pointing at a real problem visible in context.
Proper review currently requires checking component locally, resetting the baseline changes and then running the psalm. Occasionally due to runtime PHP version differences psalm will report differently which further complicates review.
I suggest we introduce two new additional psalm jobs.
Job to report newly baselined psalm errors in the current PR
No failure psalm job that would checkout targeted branch baseline to run against with the exception of explicitly ignoring unused baseline entries.
This job would be an important review tool that would allow to see exactly which problems were baselined by a given PR without having to guess from baseline diff or having to checkout code and run psalm locally.
I think this job should report found errors as check annotations on the PR as well, although with reduced severity - notice instead of error. Check annotations with the current beta feature are shown in context even for files that were not directly changed by the PR. This allows to view error in context without having to navigate to the code in a separate tab.
To distinguish real psalm errors from baselined errors this job should rewrite github formatted psalm output to report as Notice instead of Warning or Error. Custom output formatter would be right way to do it but simple sed will work just as fine, until and unless github changes format. See GHA docs: Setting a notice message and Psalm GithubActionsReport formatter
In slack discussion @Ocramius expressed skepticism about usefulness of annotations on pull request Files Changed tab over the visual noise they create. I believe annotations should be utilized for a couple reasons:
Job to report all psalm baselined errors in action log
No failure
--ignore-baselinerun with default output formater that would not show in PR as failure or as PR annotations. This job would allow to see in action log exactly which issues are currently baselined.Bonus points for reporting total number of errors found as check notice message to be displayed as annotation on PR with something like
152 baselined psalm errors found. The way github shows annotations with "View workflow job for this annotation" button would serve as a way to increase awareness for contributors and link directly at reported baselined errors without getting in the way.