Reporting module: open PR triage — 12 look safe to close, 2 reviewed and worth finishing

Development
1

Hi @mseaton,

I went through all 26 open PRs on openmrs-module-reporting (20 from contributors, 6 from Dependabot). I checked each one against current master and its REPORT ticket, and built and tested the ones that still looked relevant. Summary below, in case it saves you time.

12 look safe to close as obsolete. Their tickets are already Closed as Won’t Fix or Won’t Do: #168 (REPORT-770), #170 (REPORT-848), #171 (REPORT-839, which also targets the removed api-1.10 module), #172 (REPORT-823), #175 (REPORT-806), #189 (REPORT-602), #193 (REPORT-635), #205 (REPORT-495), #209 (REPORT-547), #210 (REPORT-868) and #230 (REPORT-870; its head branch is openmrs:master, so it has no diff). #242 (RA-2020) also belongs here: that ticket was Closed as Fixed without this PR.

6 are superseded by later work:

  • #247 (xstream whitelisting): master already has this via TRUNK-6218.
  • #267 (Codecov): it edits maven.yml, which #268 replaced with the shared workflow.
  • #196, #241, #250, #257: earlier HttpReportProcessor attempts. #258 is the current one.

2 are worth finishing. I’ve left reviews on both:

  • #274 (dkayiwa’s Platform 3.0 upgrade): it builds, and all tests pass on core 3.0.0-SNAPSHOT once the htmlwidgets and serialization.xstream PRs it depends on are installed. One fix is needed before TRUNK-6755 lands; details are in the review.
  • #258 (HttpReportProcessor, REPORT-862): it still has the content-type and config-property issues you raised on #196 in 2020. It also corrupts non-ASCII output and has no timeouts. REPORT-907 is a clone of REPORT-862 and could be closed as a duplicate.

Dependabot:

  • #234 (mysql-connector, test scope, used only by an ignored test) is safe to merge. I approved it.
  • #197 (quartz 2.3.2) is fine with three extra exclusions. Without them the .omod would bundle an older c3p0 than core ships. The review has the fix, and I can open a replacement PR.
  • #239 (jackson-databind) and #235/#211 (POI 4) aren’t safe as they stand.
  • #252 (groovy) is obsolete, since REPORT-912 removed groovyVersion.

Related: I filed REPORT-927 to move master off the deprecated OpenmrsUtil.applyLogLevel. TRUNK-6755 removes it, and it’s the only TRUNK-6755 shim reporting uses. The fix and a test are ready, and I’ll open the PR shortly.

If closing the obsolete ones is easier as a batch, I’m happy to list exactly what I checked on each.