Skip to content

Conversation

bts
Copy link
Contributor

@bts bts commented Oct 21, 2025

Ahead of subsequent changes to convert more methods on Index, MutableIndex, ReadonlyIndex, and ChangeIdIndex to be fallible, these four changes:

  • unify Index and IndexStore's disparate error types into enums like other storage traits (e.g. BackendError, OpStoreError).
  • introduce an IndexResult type alias (like those of BackendResult and OpStoreResult) that will help as more index trait methods are made fallible. We introduce IndexStoreResult while we're at it, for symmetry with the other storage trait result types.

By making IndexError an enum and returning that from Index::all_heads_for_gc, we allow for network-based implementations of this method to produce arbitrary I/O errors in addition to the existing case of AllHeadsForGcUnsupported.

Let me know if there's anything I should change -- e.g. drop the IndexStoreResult alias, submit one PR per change, etc.

Thanks!
Brian

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes

@bts bts requested a review from a team as a code owner October 21, 2025 01:50
bts added 4 commits October 22, 2025 15:22
This way we match how errors are organized for the other storage traits,
and we allow for networked implementations of `Index::all_heads_for_gc`
to return I/O errors.
This way we match how the errors are organized for other storage traits.
By using a type alias, we match how `Backend` and `OpStore` traits
organize their errors. This will be useful as we progressively convert
more methods on `Index`, `MutableIndex`, `ReadonlyIndex`, and
`ChangeIdIndex` traits to return `Result<T, IndexError>` in order to
support networked implementations which can produce I/O failures.
By introducing `IndexStoreResult`, we follow the pattern used by other
storage traits: `BackendResult`, `OpStoreResult`, and `IndexResult`.
@martinvonz
Copy link
Member

@bts: In case you missed it, I invited you to the contributors team yesterday

@bts
Copy link
Contributor Author

bts commented Oct 23, 2025

Great -- thank you, Martin!

@bts bts added this pull request to the merge queue Oct 23, 2025
Merged via the queue into jj-vcs:main with commit 2d1acbe Oct 23, 2025
29 checks passed
@bts bts deleted the bts/index-error-enums branch October 23, 2025 02:28
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.

2 participants