Move MentatError away from the top-level crate #805
Conversation
|
This introduces a temporary inconsistency in how crates are named - the new crates I've added don't use the "mentat_" prefix. The intention is to remove that prefix entirely, but that will be done once work that depends on this - in #563 - lands, to avoid an even more annoying rebase. |
There was a problem hiding this comment.
This looks great. I expect it to have changed the public API, so please bump the Mentat version (to 0.12.0, and don't forget the version in sdks/android).
I'm surprised that you don't have a src/ directory in *-traits. Is there a reason?
Otherwise, bombs away! Good work.
src/errors.rs
Outdated
| @@ -72,13 +72,13 @@ pub enum MentatError { | |||
| InvalidVocabularyVersion, | |||
|
|
|||
| #[fail(display = "vocabulary {}/{} already has attribute {}, and the requested definition differs", _0, _1, _2)] | |||
| ConflictingAttributeDefinitions(String, ::vocabulary::Version, String, Attribute, Attribute), | |||
There was a problem hiding this comment.
In all of these messages, ensure that all of these u32 values are printed with "version {}" so that it's clear what the integer means. So
#[fail(display = "vocabulary {}/version {} already has attribute {}, and the requested definition differs", _0, _1, _2)]
|
I don't expect the public API to have changed - but it's probably a fair statement that it did in some subtle way, and we currently don't have much in place to ensure that it didn't change. Regarding the lack of |
For some reason, the converted doc test fails on Rust 1.25.0, while working with other Rust versions. For simplicity, just convert it into a regular test.
grigoryk
force-pushed
the
grisha/type_composition
branch
from
5f9f6e6
to
a7bb805
Compare
This PR is in preparation for landing some of the work in #563 - specifically, the Pre work which splits
transaction-like functionality away from the top-level into its own inner crate.Precommits move any trait or type that is necessary to compose an error out of a given crate, and into a "leaf" crate, which use the*-traitsnaming convention (e.g.dbanddb-traits). This pattern is similar to how Servo structures their internal inter-dependencies, although at a weaker level (since I'm retrofitting this pattern onto the current world, and going all-in on it would be quite a bit of work!).This move is needed so that we can use a single Error type in our public API.
Without these changes, the Pre patch above would have to leak a
TransactionErrorinto mentat's public API space, which is a subset of theMentatError. If theMentatErroris defined in an inner crate, it can now be referenced in other inner crates, in absence of circular dependencies.This pattern is useful for crates which:
A prime example is the "mentat transaction state manager" (the
transactioncrate in #563), and the upcomingtolstoywhich makes heavy use of thetransactioncrate.