Skip to content

Commit 07866ca

Browse files
joewizclaude
joewiz
and
claude
committed
[doc] Update v2 reviewer guide: fix Jetty PR ref, add CI health note, update statuses
- Replace closed eXist-db#6213 (v2/jetty-12-upgrade) with eXist-db#6145 (feature/websocket-core) - Add CI Health Note explaining known noise: integration hangs, container image HTTP 502, XQTS runner Saxon 12 crash, and complementary empty-match failures in eXist-db#6212/eXist-db#6218 - Update XQTS runner: eXist-db#45 closed, eXist-db#49 is the active PR - Update cross-repo PR table accordingly - Update "Also Ready to Merge" table: mark eXist-db#6142, eXist-db#6146 merged; eXist-db#6186 superseded by eXist-db#6224; correct eXist-db#6087 approver; add status notes for eXist-db#6182, eXist-db#6184 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b543b47 commit 07866ca

1 file changed

Lines changed: 38 additions & 29 deletions

File tree

‎taskings/v2-reviewer-guide.md‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Overview
44

5-
This is a guide to the 14 `v2/` PRs prepared for eXist-db 7.0. Each PR has been individually tested (build, exist-core tests, Codacy), and all 15 merge cleanly together in the `next-v2` integration branch.
5+
This is a guide to the 13 `v2/` PRs (plus one closely related PR, [#6145](https://github.com/eXist-db/exist/pull/6145)) prepared for eXist-db 7.0. Each PR has been individually tested (build, exist-core tests, Codacy), and all branches merge cleanly together in the `next-v2` integration branch.
66

77
The PRs are organized into 5 waves by dependency and review complexity. **You can review in any order**, but merging should follow the wave order to avoid conflicts.
88

@@ -12,6 +12,17 @@ Every branch was tested individually: build (`mvn install -pl exist-core -am`),
1212

1313
For full context on the consolidation process, see the [consolidation report](v2-consolidation-report.md).
1414

15+
## CI Health Note
16+
17+
All 13 `v2/` PRs were rebased on 2026-04-13 after [#6224](https://github.com/eXist-db/exist/pull/6224) (CI/Maven fixes) merged. The CI runs from that rebase are the current baseline.
18+
19+
**Known noise in CI results** — do not treat these as blockers:
20+
21+
- **Integration failures (ubuntu/windows/macOS)**: All PRs show 1–3 integration job failures. These are pre-existing test hangs (the surefire fork timeout fires; the CI job still reports FAILURE). Not caused by any v2 change.
22+
- **"Test and Publish Container Images" failure**: Caused by a transient HTTP 502 from `exist-db.org`'s public XAR repo during the build. Not a code issue.
23+
- **XQTS runner crash on #6212 (Saxon 12)**: The CI XQTS job uses the Saxon 9.9 runner against the Saxon 12 classpath and crashes with `NoSuchMethodError: AnyURIValue.<init>(CharSequence)`. This is expected; [exist-xqts-runner #49](https://github.com/eXist-db/exist-xqts-runner/pull/49) adds Saxon 12 compatibility and should merge alongside #6212.
24+
- **`replace.empty-match` unit tests (#6212 and #6218)**: These two PRs have complementary unit test failures. #6212 has `replace.empty-match-fails` failing (Saxon 12 permits empty matches; the XQ 3.1 gating is in #6218). #6218 has `replace.empty-match-allowed` failing (XQ4 mode expects empty matches; requires Saxon 12 from #6212). When both are merged together, both tests pass. Reviewers can ignore these failures when reviewing either PR individually.
25+
1526
## How to Test
1627

1728
Pull the Docker image with all branches pre-merged:
@@ -105,15 +116,18 @@ Eliminates the `exist-saxon-regex` fork module entirely. The main work is migrat
105116

106117
---
107118

108-
### `v2/jetty-12-upgrade` ([#6213](https://github.com/eXist-db/exist/pull/6213)) — Jetty 11 → 12 (Jakarta Servlet 6.0)
119+
### `feature/websocket-core` ([#6145](https://github.com/eXist-db/exist/pull/6145)) — Jetty 11 → 12 (Jakarta Servlet 6.0) + WebSocket
109120

110121
**Priority**: High — Jetty 11 is EOL
111-
**Reviewer effort**: Medium (2 commits, 41 files)
122+
**Reviewer effort**: Medium (~41 files for Jetty migration + WebSocket module)
112123
**Risk**: Medium — servlet API migration affects all HTTP handling
124+
**Approvals**: @dizzzz ✓
125+
126+
`javax.servlet` → `jakarta.servlet` across all modules. Addresses review feedback from @reinhapa and @dizzzz on the original PR #6144: `printStackTrace` replaced with logging, unhelpful comments removed, `sendError` evaluated but not used for RESTXQ (would commit response prematurely). Also adds a WebSocket module with streaming XQuery evaluation.
113127

114-
`javax.servlet` → `jakarta.servlet` across all modules. Addresses review feedback from @reinhapa and @dizzzz on the original PR #6144: `printStackTrace` replaced with logging, unhelpful comments removed, `sendError` evaluated but not used for RESTXQ (would commit response prematurely).
128+
> Note: The original `v2/jetty-12-upgrade` PR ([#6213](https://github.com/eXist-db/exist/pull/6213)) has been superseded by this broader PR.
115129
116-
**What to look for**: The `setStatus` reason parameter decision (dropped, not replaced with `sendError` — see comment in HttpServletResponseAdapter.java). Servlet filter chain changes. WebSocket compatibility.
130+
**What to look for**: The `setStatus` reason parameter decision (dropped, not replaced with `sendError` — see comment in HttpServletResponseAdapter.java). Servlet filter chain changes. WebSocket API.
117131

118132
---
119133

@@ -233,7 +247,7 @@ Wave 1 (any order): v2/xq31-compliance-fixes
233247
v2/xq4-filter-expr-am
234248
235249
Wave 2 (any order): v2/saxon-12-upgrade
236-
v2/jetty-12-upgrade
250+
feature/websocket-core (#6145)
237251
238252
Wave 3 (in order): v2/w3c-xquery-update-3.0
239253
v2/xqft-phase2
@@ -250,16 +264,11 @@ Wave 5 (any order): v2/serialization-compliance
250264

251265
### XQTS Runner (exist-xqts-runner repo)
252266

253-
[PR #45](https://github.com/eXist-db/exist-xqts-runner/pull/45) — "Add QT4/FTTS test suites, XQuery Update support, and assertion reliability fixes" (approved by @duncdrum)
267+
[PR #49](https://github.com/eXist-db/exist-xqts-runner/pull/49) — "Extend XQTS runner: QT4/FTTS/Update suites, assertion fixes, batch runner, Saxon 12"
254268

255-
This PR enables all the XQTS compliance scores cited in this guide. Without it, only XQ 3.1 tests can be run. It adds:
256-
- QT4 test suite support (XQuery 4.0)
257-
- FTTS test suite support (Full Text)
258-
- XQuery Update feature flag
259-
- Batch runner with parallel execution and jstack diagnostics
260-
- Saxon 12 / Jetty 12 compatibility
269+
This consolidated PR supersedes the previously approved [#45](https://github.com/eXist-db/exist-xqts-runner/pull/45) (now closed). It enables all the XQTS compliance scores cited in this guide and adds Saxon 12 compatibility. Without it, only XQ 3.1 tests can be run.
261270

262-
**Should be merged alongside the v2/ PRs.**
271+
**Should be merged alongside the v2/ PRs.** Needs first review.
263272

264273
### W3C XInclude Test Suite
265274

@@ -271,8 +280,8 @@ These PRs on other repositories are part of the 7.0 work:
271280

272281
| Repo | PR | Title | Status |
273282
|------|----|-------|--------|
274-
| exist-xqts-runner | [#45](https://github.com/eXist-db/exist-xqts-runner/pull/45) | QT4/FTTS/XQUF test suite support | Approved (@duncdrum) |
275-
| exist-xqts-runner | [#49](https://github.com/eXist-db/exist-xqts-runner/pull/49) | Saxon 12 compatibility | Merge after v2/saxon-12-upgrade |
283+
| exist-xqts-runner | [#49](https://github.com/eXist-db/exist-xqts-runner/pull/49) | QT4/FTTS/XQUF suites + Saxon 12 | Needs review; merge alongside v2/ PRs |
284+
| exist-xqts-runner | [~~#45~~](https://github.com/eXist-db/exist-xqts-runner/pull/45) | ~~QT4/FTTS/XQUF test suite support~~ | Closed; superseded by #49 |
276285
| eXist-db/exist | [#6206](https://github.com/eXist-db/exist/pull/6206) | XInclude test suite + conformance | Review needed |
277286
| eXide | [#778](https://github.com/eXist-db/eXide/pull/778) | Modernize: CM6 editor, REx parser, LSP | Review needed |
278287
| exist-markdown | [#69](https://github.com/eXist-db/exist-markdown/pull/69) | CommonMark/GFM (flexmark-java) | Approved |
@@ -305,18 +314,18 @@ These repos were created for 7.0 apps and should be transferred to the eXist-db
305314

306315
These are not part of the v2/ consolidation but are approved and ready:
307316

308-
| PR | Title | Approvals |
309-
|----|-------|-----------|
310-
| [#6087](https://github.com/eXist-db/exist/pull/6087) | Fix XInclude relative path resolution | @reinhapa |
311-
| [#6092](https://github.com/eXist-db/exist/pull/6092) | [ZN] timezone name modifier for format-dateTime | @duncdrum |
312-
| [#6142](https://github.com/eXist-db/exist/pull/6142) | Add weekly Prethink context refresh workflow | @line-o |
313-
| [#6146](https://github.com/eXist-db/exist/pull/6146) | Lucene 10 upgrade | @windauer, @line-o |
314-
| [#6153](https://github.com/eXist-db/exist/pull/6153) | Re-enable 9 skipped tests that now pass | (merged) |
315-
| [#6162](https://github.com/eXist-db/exist/pull/6162) | Fix negative double/float in value index | @reinhapa |
316-
| [#6163](https://github.com/eXist-db/exist/pull/6163) | Validate json-to-xml escape option (FOJS0005) | @duncdrum |
317-
| [#6182](https://github.com/eXist-db/exist/pull/6182) | Unify module discovery | @line-o |
318-
| [#6184](https://github.com/eXist-db/exist/pull/6184) | Add repo:resource-available() | @duncdrum |
319-
| [#6186](https://github.com/eXist-db/exist/pull/6186) | Add surefire fork timeouts | @duncdrum |
320-
| [#6191](https://github.com/eXist-db/exist/pull/6191) | Fix FLWOR sort race condition | @duncdrum |
317+
| PR | Title | Status |
318+
|----|-------|--------|
319+
| [#6087](https://github.com/eXist-db/exist/pull/6087) | Fix XInclude relative path resolution | 1 approval (@duncdrum) |
320+
| [#6092](https://github.com/eXist-db/exist/pull/6092) | [ZN] timezone name modifier for format-dateTime | 1 approval (@duncdrum) |
321+
| [~~#6142~~](https://github.com/eXist-db/exist/pull/6142) | ~~Add weekly Prethink context refresh workflow~~ | Merged 2026-04-13 |
322+
| [~~#6146~~](https://github.com/eXist-db/exist/pull/6146) | ~~Lucene 10 upgrade~~ | Merged 2026-04-13 |
323+
| [~~#6153~~](https://github.com/eXist-db/exist/pull/6153) | ~~Re-enable 9 skipped tests that now pass~~ | Merged |
324+
| [#6162](https://github.com/eXist-db/exist/pull/6162) | Fix negative double/float in value index | 2 approvals (@duncdrum, @reinhapa) — ready to merge |
325+
| [#6163](https://github.com/eXist-db/exist/pull/6163) | Validate json-to-xml escape option (FOJS0005) | 2 approvals (@duncdrum, @reinhapa) — ready to merge |
326+
| [#6182](https://github.com/eXist-db/exist/pull/6182) | Unify module discovery | 1 approval (@duncdrum); @adamretter has raised a concern about benchmark methodology |
327+
| [#6184](https://github.com/eXist-db/exist/pull/6184) | Add repo:resource-available() | 1 approval (@duncdrum); changes requested by @line-o |
328+
| [~~#6186~~](https://github.com/eXist-db/exist/pull/6186) | ~~Add surefire fork timeouts~~ | Closed; superseded by [#6224](https://github.com/eXist-db/exist/pull/6224) (merged 2026-04-13) |
329+
| [#6191](https://github.com/eXist-db/exist/pull/6191) | Fix FLWOR sort race condition | 1 approval (@duncdrum) — ready to merge |
321330

322331
These can be merged at any time — they don't conflict with the v2/ branches.

0 commit comments

Comments
 (0)