affinity-intelligence-rework/im2be-protobuf
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

docs(avro): correct schema_version bump trigger (not additions-only) (#292) #5

Merged
hibryda merged 2 commits from chore/avro-readme-schema-version-phrasing into main 2026-05-28 20:42:39 +02:00
Conversation 2 Commits 2 Files changed 1 +1 -1
hibryda commented 2026-05-28 20:35:25 +02:00
Owner
Copy link

avro/README.md schema_version phrasing fix (#292)

TL;DR — CONDITIONAL (docs-only). avro/README.md:34 said schema_version is "bumped on backward-compatible additions" — too narrow. The Backward-compatibility rules section immediately below (Apicurio BACKWARD mode) lists compatible evolutions that are not additions — notably removing a defaulted field. Reworded to "each backward-compatible schema change" + enumerate the cases, matching the section and removing the misleading additions-only constraint (same correction previously applied to the v1 doc string).

Change

  • avro/README.md line 34 — one-line doc reword. No schema/code changes.

Verdict

CONDITIONAL — docs-only consistency fix.

protobuf • #292 • base main • 2026-05-28

## avro/README.md schema_version phrasing fix (#292) **TL;DR — `CONDITIONAL` (docs-only).** `avro/README.md:34` said `schema_version` is "bumped on backward-compatible **additions**" — too narrow. The *Backward-compatibility rules* section immediately below (Apicurio BACKWARD mode) lists compatible evolutions that are **not** additions — notably **removing a defaulted field**. Reworded to "each backward-compatible schema change" + enumerate the cases, matching the section and removing the misleading additions-only constraint (same correction previously applied to the v1 doc string). ### Change - `avro/README.md` line 34 — one-line doc reword. No schema/code changes. ### Verdict `CONDITIONAL` — docs-only consistency fix. --- *protobuf • #292 • base main • 2026-05-28*
docs(avro): correct schema_version bump trigger — backward-compatible changes, not only additions
Some checks failed
protobuf CI / scripts/check-import-direction.sh (pull_request) Successful in 2s
Details
protobuf CI / buf lint (pull_request) Failing after 5s
Details
protobuf CI / buf build (pull_request) Failing after 4s
Details
27db7578de 
avro/README.md line 34 said schema_version is "bumped on backward-compatible
additions" — too narrow. The Backward-compatibility rules section directly below
(Apicurio BACKWARD mode) lists compatible evolutions that are NOT additions —
notably REMOVING a defaulted field. Reworded to "each backward-compatible schema
change" + enumerate the compatible cases, matching the section and removing the
misleading additions-only constraint (same fix applied to the v1 doc string).
pr-reviewer-bot commented 2026-05-28 20:37:36 +02:00
Member
Copy link

Superseded by round 2.

Show previous round

hib-pr-reviewer review — PR #5 (affinity-intelligence-rework/im2be-protobuf)

Round 1 — head 27db7578de72, base main, trigger opened

TL;DR: CONDITIONAL_APPROVE — kept 1 minor unique-to-B finding (verified); Reviewer A agreed there are no blocking issues; drift risk from duplicated inline enumeration is real but non-blocking.

Summary

Arbitration Summary

No prior run history found (first arbitration for this PR). No reusable submodule patterns stored in Memora.

Verification performed: Read avro/README.md at HEAD to check both the changed line 34 and the referenced Backward-compatibility rules section (lines 41–43).

Reviewer A raised 0 findings. Reviewer B raised 1 minor finding (unique-to-B), which was verified:

  • Line 34 (the changed line) confirms the new parenthetical enumerates all three items: adding a defaulted field, adding a ["null", …] union arm, or removing a defaulted field.
  • Lines 41–43 confirm those are exactly the three ✅ items in the Backward-compatibility rules section.

B's analysis is grounded: two canonical lists now exist in the same file. If a future contributor appends a fourth ✅ rule (e.g. widening an enum via aliases) to lines 41–43, line 34 will silently go stale. Finding kept at its original minor severity. B's suggested replacement (cross-reference by section name rather than re-listing) is a valid remediation.

Blast Radius

Single documentation file (avro/README.md) changed, no code, schemas, or CI configs touched. The only risk is human misreading of documentation; no runtime or schema-registry behaviour is affected.

BLAST_SCORE: 1/10

CI status (head 27db7578de72)

Overall: ✗ failure

3 checks: 3 pending

Check State Link
protobuf CI / scripts/check-import-direction.sh (pull_request) ⏳ pending details
protobuf CI / buf lint (pull_request) ⏳ pending details
protobuf CI / buf build (pull_request) ⏳ pending details

Findings (1)

[MINOR] Inline enumeration duplicates the Backward-compatibility rules section — drift risk

avro/README.md:34

The new parenthetical (adding a defaulted field, adding a \["null", …]` union arm, or removing a defaulted field)is a verbatim re-listing of all three ✅ items in the *Backward-compatibility rules* section (lines 41–43). This creates two sources of truth: a contributor who later adds a new compatible evolution (e.g. widening an enum viaaliases`) to lines 41–43 is unlikely to also update the inline list at line 34, causing it to silently go stale.

Verified at HEAD: line 34 contains exactly the three-item parenthetical; lines 41–43 contain the matching ✅ bullets.

Suggested fix — replace the parenthetical with a section cross-reference to avoid duplication:

- **`schema_version`**: explicit int field separate from the topic's `.vN` suffix; bumped on each backward-compatible schema change listed under *Backward-compatibility rules* below — not only additions.

Verdict

CONDITIONAL_APPROVE

hib-pr-reviewer • round 1 • 1 finding (1m) • 2026-05-28T18:36:36.869Z → 2026-05-28T18:37:36.494Z • posted-as: pr-reviewer-bot

<!-- hib-pr-reviewer collapsed --> > _Superseded by round 2._ <details> <summary>Show previous round</summary> ## hib-pr-reviewer review — PR #5 (affinity-intelligence-rework/im2be-protobuf) **Round 1** — head `27db7578de72`, base `main`, trigger `opened` **TL;DR:** CONDITIONAL_APPROVE — kept 1 minor unique-to-B finding (verified); Reviewer A agreed there are no blocking issues; drift risk from duplicated inline enumeration is real but non-blocking. ### Summary ## Arbitration Summary No prior run history found (first arbitration for this PR). No reusable submodule patterns stored in Memora. **Verification performed:** Read `avro/README.md` at HEAD to check both the changed line 34 and the referenced Backward-compatibility rules section (lines 41–43). **Reviewer A** raised 0 findings. **Reviewer B** raised 1 minor finding (unique-to-B), which was verified: - Line 34 (the changed line) confirms the new parenthetical enumerates all three items: *adding a defaulted field*, *adding a `["null", …]` union arm*, **or** *removing a defaulted field*. - Lines 41–43 confirm those are exactly the three ✅ items in the *Backward-compatibility rules* section. B's analysis is grounded: two canonical lists now exist in the same file. If a future contributor appends a fourth ✅ rule (e.g. widening an enum via `aliases`) to lines 41–43, line 34 will silently go stale. Finding kept at its original **minor** severity. B's suggested replacement (cross-reference by section name rather than re-listing) is a valid remediation. ### Blast Radius Single documentation file (`avro/README.md`) changed, no code, schemas, or CI configs touched. The only risk is human misreading of documentation; no runtime or schema-registry behaviour is affected. **BLAST_SCORE: 1/10** ### CI status (head `27db7578de72`) **Overall: ✗ failure** 3 checks: 3 pending | Check | State | Link | |---|---|---| | protobuf CI / scripts/check-import-direction.sh (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/19/jobs/0) | | protobuf CI / buf lint (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/19/jobs/1) | | protobuf CI / buf build (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/19/jobs/2) | ### Findings (1) #### **[MINOR]** Inline enumeration duplicates the Backward-compatibility rules section — drift risk _avro/README.md:34_ The new parenthetical `(adding a defaulted field, adding a \`["null", …]\` union arm, **or removing a defaulted field**)` is a verbatim re-listing of all three ✅ items in the *Backward-compatibility rules* section (lines 41–43). This creates two sources of truth: a contributor who later adds a new compatible evolution (e.g. widening an enum via `aliases`) to lines 41–43 is unlikely to also update the inline list at line 34, causing it to silently go stale. Verified at HEAD: line 34 contains exactly the three-item parenthetical; lines 41–43 contain the matching ✅ bullets. **Suggested fix** — replace the parenthetical with a section cross-reference to avoid duplication: ```md - **`schema_version`**: explicit int field separate from the topic's `.vN` suffix; bumped on each backward-compatible schema change listed under *Backward-compatibility rules* below — not only additions. ``` ### Verdict **CONDITIONAL_APPROVE** --- <sub>hib-pr-reviewer • round 1 • 1 finding (1m) • 2026-05-28T18:36:36.869Z → 2026-05-28T18:37:36.494Z • posted-as: pr-reviewer-bot</sub> </details>
docs(avro): R1 — reference Backward-compatibility rules instead of duplicating the list (#5)
Some checks failed
protobuf CI / scripts/check-import-direction.sh (pull_request) Successful in 3s
Details
protobuf CI / buf lint (pull_request) Failing after 7s
Details
protobuf CI / buf build (pull_request) Failing after 4s
Details
d6bd36c8c8 
R1 verdict CONDITIONAL_APPROVE — 1 MINOR (unique-to-B), applied:

(1) MINOR — the line re-enumerated the compatible cases inline (add defaulted
    field / union arm / remove defaulted field), duplicating the
    *Backward-compatibility rules* section directly below → drift risk if the
    section changes. Dereferenced: now points at the section without re-listing,
    keeping only the "not only additions" correction.
pr-reviewer-bot commented 2026-05-28 20:39:57 +02:00
Member
Copy link

hib-pr-reviewer review — PR #5 (affinity-intelligence-rework/im2be-protobuf)

Round 2 — head d6bd36c8c818, base main, trigger synchronize

TL;DR: NO_NEW_FINDINGS — No new findings this round.

Summary

[quorum-converged] A=0 = B=0. The round-2 synchronize push directly addresses the round-1 finding. The old bullet text (bumped on backward-compatible additions) stated only field additions as the trigger; the new text (bumped on each backward-compatible schema change (the cases enumerated under *Backward-compatibility rules* below), not only field additions) does two things correctly:

  1. Factual fix — the authoritative section at lines 41–46 lists three ✅ triggers, of which only one is a field addition (removing a field that had a default is also backward-compatible and should bump schema_version). The corrected phrasing is accurate.
  2. Drift eliminated — instead of inlining a separate enumeration that could diverge from the section below, it now cross-references *Backward-compatibility rules*, which is the exact heading at line 37. The round-1 concern no longer applies.

No new actionable issues were found in this diff.

CI status (head d6bd36c8c818)

Overall: ✗ failure

3 checks: 3 pending

Check State Link
protobuf CI / scripts/check-import-direction.sh (pull_request) ⏳ pending details
protobuf CI / buf lint (pull_request) ⏳ pending details
protobuf CI / buf build (pull_request) ⏳ pending details

Findings

No new findings this round.

Quorum converged on empty findings (A + B both returned 0).

Verdict

NO_NEW_FINDINGS

hib-pr-reviewer • round 2 • 0 findings • 2026-05-28T18:39:13.477Z → 2026-05-28T18:39:56.831Z • posted-as: pr-reviewer-bot • [bookkeeping fallback]

## hib-pr-reviewer review — PR #5 (affinity-intelligence-rework/im2be-protobuf) **Round 2** — head `d6bd36c8c818`, base `main`, trigger `synchronize` **TL;DR:** NO_NEW_FINDINGS — No new findings this round. ### Summary [quorum-converged] A=0 = B=0. The round-2 synchronize push directly addresses the round-1 finding. The old bullet text (`bumped on backward-compatible additions`) stated only field additions as the trigger; the new text (`bumped on each backward-compatible schema change (the cases enumerated under *Backward-compatibility rules* below), not only field additions`) does two things correctly: 1. **Factual fix** — the authoritative section at lines 41–46 lists three ✅ triggers, of which only one is a field addition (removing a field that had a default is also backward-compatible and should bump `schema_version`). The corrected phrasing is accurate. 2. **Drift eliminated** — instead of inlining a separate enumeration that could diverge from the section below, it now cross-references `*Backward-compatibility rules*`, which is the exact heading at line 37. The round-1 concern no longer applies. No new actionable issues were found in this diff. ### CI status (head `d6bd36c8c818`) **Overall: ✗ failure** 3 checks: 3 pending | Check | State | Link | |---|---|---| | protobuf CI / scripts/check-import-direction.sh (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/20/jobs/0) | | protobuf CI / buf lint (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/20/jobs/1) | | protobuf CI / buf build (pull_request) | ⏳ pending | [details](/affinity-intelligence-rework/im2be-protobuf/actions/runs/20/jobs/2) | ### Findings **No new findings this round.** _Quorum converged on empty findings (A + B both returned 0)._ ### Verdict **NO_NEW_FINDINGS** --- <sub>hib-pr-reviewer • round 2 • 0 findings • 2026-05-28T18:39:13.477Z → 2026-05-28T18:39:56.831Z • posted-as: pr-reviewer-bot • [bookkeeping fallback]</sub>
hibryda deleted branch chore/avro-readme-schema-version-phrasing 2026-05-28 20:42:39 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
affinity-intelligence-rework/im2be-protobuf!5
No description provided.