Skip to content

FIX: Small code-review fixes + FAQ section - #12

Merged
kishorereddy merged 3 commits into
mainfrom
fix/260803-fixes
Aug 4, 2026
Merged

FIX: Small code-review fixes + FAQ section#12
kishorereddy merged 3 commits into
mainfrom
fix/260803-fixes

Conversation

@kishorereddy

Copy link
Copy Markdown
Contributor

Overview

  1. Code review cleanup: four small correctness/design fixes from a code review pass.
  2. New FAQ section: added to the README, covering design rationale and common questions.

Ticket(s)

N/A

Link(s)

N/A

Example(s)

// Rejected requests now map to a client-side HTTP code, not a server-fault one
CodesToHttp().toCode(Rejected.RULE_VIOLATION) // 409, was 500

// Override maps now match by identity (origin+name), not full field equality
val differentMessage = Failed.Invalid("NOT_FOUND", "a different message", origin = "kiit")
CodesToHttp().toCode(differentMessage) // still resolves to 404

Dependencies

None.

Design

  1. Removed copyAll: Status no longer exposes a general-purpose copy method. Its one real
    dependent, Status.ofStatus, still works the same way internally, just without a public mutation
    escape hatch.
  2. Removed isNeutral: a computed convenience that read as contradictory in practice
    (Excluded being both success = true and "neutral" at once). is Passed.Excluded || is Passed.Information is still trivially available inline.
  3. Fixed Rejected's HTTP default: 500 → 409: a well-formed, understood request that's
    refused on a business rule is a client-side outcome, not a server fault.
  4. Override maps now key on identity, not full equality: Status is a data class, so the old
    Map<Status, Int> overrides compared every field, including the message. Two statuses with the
    same identity but a slightly different message could silently miss their override. Rekeyed to
    match on origin + name instead.
  5. Confirmed gRPC UNSUPPORTED12: previously flagged as an inference; now confirmed and
    documented as such.
  6. New FAQ section in the README covering design philosophy, comparisons to alternatives
    (Arrow, kotlin-result), taxonomy details, adoption guidance, and maturity/trust questions.

Notes

None.

Pending

None.

Tests

  1. Updated tests for the HTTP default change (409 instead of 500) and the resulting shift in which
    codes tie at 409 vs 500.
  2. Added regression tests confirming override maps and CompositeLookup match by identity even
    when the message differs.
  3. Removed tests that only existed to exercise copyAll/isNeutral directly.
  4. Full ktlintCheck + detekt + jvmTest + jsNodeTest run clean.

@kishorereddy
kishorereddy merged commit 144c888 into main Aug 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant