edgedb-rust icon indicating copy to clipboard operation
edgedb-rust copied to clipboard

Choose appropriate name for the `edgedb-protocol::model` module

Open tailhook opened this issue 5 years ago • 10 comments

Previous discussion: https://github.com/edgedb/edgedb-rust/pull/44 and https://github.com/edgedb/edgedb-rust/issues/38

Options so far:

  1. data_types is what is used in Python (or rather datatypes), but can be perceived to long for referencing too often
  2. Keep it model (the name was was not discussed beforehand so should not be considered status quo)
  3. data is shorter for data types
  4. scalars -- but this might stop using it for more complex types if we need in future
  5. Make it a separate crate, e.g. edgedb-types

Personally, I still like model, but data is also fine.

tailhook avatar Aug 19 '20 11:08 tailhook

I don't like scalars, since that's a technical detail, users won't care about (and some might not even know what a scalar is).

I like model and data best, but data_types is okay as well.

CodesInChaos avatar Aug 19 '20 12:08 CodesInChaos

I think model is too close to schema, which is the term we actually use. Can we define the exact purpose of the module before we choose the name? If it's intended to provide interfaces to EdgeDB schema and not just scalar type implementations, then schema would be more appropriate. If it's just for primitive types, then something other than model to avoid confusion with a hypothetical future schema module.

elprans avatar Aug 19 '20 16:08 elprans

Conceptually it should contain edgedb types a statically typed application would use in its data model (as opposed to edgedb-cli which is mostly dynamically typed via Value).

Currently they're all scalars and I see no clear demand for complex types. But it feels like co-incidence for me. I could conceive of types, like a streaming set or higher level representations of links eventually ending up there.

CodesInChaos avatar Aug 19 '20 16:08 CodesInChaos

Hm, can we actually call the top module 'schema' and then nest 'std' and 'cal' into it? So schema::std::int64 and schema::cal::local_date? Kind of reflecting the actual layout of EdgeDB to Rust in a 1-to-1 fashion.

1st1 avatar Aug 19 '20 19:08 1st1

Hm, can we actually call the top module 'schema' and then nest 'std' and 'cal' into it? So schema::std::int64 and schema::cal::local_date? Kind of reflecting the actual layout of EdgeDB to Rust in a 1-to-1 fashion.

  1. It's unexpected in Rust to use underscore names for types
  2. It's a very long name: edgedb_protocol::schema::std::int64. And can't be used as a module too, as use edgedb_protocol::schema::std conflicts with ::std. (importing schema is an option, but I do think schema::std::int64 is also too long)
  3. Custom types aren't magically appear under schema::default anyway

So technically we can, and while it can assist some auto-generating code for schema, I'm -0 on this being default for users.

tailhook avatar Aug 20 '20 09:08 tailhook

I'm -0 on this being default for users.

Fair enough.

It's a very long name: edgedb_protocol::schema::std::int64.

Speaking of long names, I'd very much prefer the Rust crate/namespace for the EdgeDB driver to be simply named edgedb::, not edgedb_protocol::. Similarly to our JS & Python drivers.

1st1 avatar Aug 21 '20 01:08 1st1

As for what to rename edgedb-protocol::model, I'm +1 to rename "model" to "data".

1st1 avatar Aug 21 '20 01:08 1st1

It's a very long name: edgedb_protocol::schema::std::int64.

Speaking of long names, I'd very much prefer the Rust crate/namespace for the EdgeDB driver to be simply named edgedb::, not edgedb_protocol::. Similarly to our JS & Python drivers.

Sure. The plan is to do it eventually. It will probably be the crate that re-exports all common things from fine-grained crates (e.g. this is how rand and some other crates are structured). The feature of postponing that, is that, if we fail at some interface and want to change it later, it will be much easier to gradually migrate from edgedb_client::Client to edgedb::Client than between multiple versions of the same crate. (note: even a single type or function signature change could trigger an incompatibility -> major version bump -> incompatibility of all the things that use this library).

tailhook avatar Aug 21 '20 08:08 tailhook

edgedb-derive causes some complications with that. It has to choose the path from which to import types used by the derived code.

  • Currently it hardcodes edgedb-protocol. This means any crate using the derive will have to import edgedb-protocol directly, in addition to edbedb_client and its re-exports.
  • If it imported edgedb_client and its re-exports, then you couldn't use derive without taking a dependency on the client
  • You could find some trick to switch between the two possible dependencies, but can't think of a way that isn't ugly. proc-macro-crate might help with that.

CodesInChaos avatar Aug 21 '20 09:08 CodesInChaos

edgedb-derive causes some complications with that. It has to choose the path from which to import types used by the derived code.

We'll see. It's not obviously we will need this. We also can ship edgedb::Queryable which is different from what edgedb_derive::Queryable is.

tailhook avatar Aug 21 '20 15:08 tailhook