Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Choose appropriate name for the edgedb-protocol::model module #46

Open
tailhook opened this issue Aug 19, 2020 · 10 comments
Open

Choose appropriate name for the edgedb-protocol::model module #46

tailhook opened this issue Aug 19, 2020 · 10 comments

Comments

@tailhook
Copy link
Contributor

tailhook commented Aug 19, 2020

Previous discussion: #44 and #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.

@CodesInChaos
Copy link
Contributor

CodesInChaos commented Aug 19, 2020

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.

@elprans
Copy link
Member

elprans commented Aug 19, 2020

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.

@CodesInChaos
Copy link
Contributor

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.

@1st1
Copy link
Member

1st1 commented Aug 19, 2020

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.

@tailhook
Copy link
Contributor Author

tailhook commented Aug 20, 2020

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.

@1st1
Copy link
Member

1st1 commented Aug 21, 2020

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
Copy link
Member

1st1 commented Aug 21, 2020

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

@tailhook
Copy link
Contributor Author

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).

@CodesInChaos
Copy link
Contributor

CodesInChaos commented Aug 21, 2020

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.

@tailhook
Copy link
Contributor Author

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.

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

No branches or pull requests

4 participants