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

model module part 2 #44

Merged
merged 1 commit into from
Aug 19, 2020
Merged

model module part 2 #44

merged 1 commit into from
Aug 19, 2020

Conversation

CodesInChaos
Copy link
Contributor

  • Adapt all internal consumers to use the types through model
  • Don't re-export from value
  • Turn modules into sub-modules of model

@1st1
Copy link
Member

1st1 commented Aug 18, 2020

I'm curious why is it called "model"? As in "data model"? Why not "types"?

@CodesInChaos
Copy link
Contributor Author

Because it's the types the application is expected to use in its data model. Just types sounded too general.

But I'm not particularly married to the name, my main goal here is to centralize all those types in one module.

@elprans
Copy link
Member

elprans commented Aug 18, 2020

Let's use datatypes for consistency with edgedb-python

@CodesInChaos
Copy link
Contributor Author

CodesInChaos commented Aug 18, 2020

You might also want to take a look at #38 and provide some feedback on naming and structure there.

Concerning datatypes, the rust naming convention of modules is snake_case, so data_types might fit better. Personally I prefer a shorter name, since this probably the most commonly used import.

@1st1
Copy link
Member

1st1 commented Aug 18, 2020

You might also want to take a look at #38 and provide some feedback on naming and structure there.

Commented there.

@tailhook
Copy link
Contributor

I also don't like data_types because of length. scalars is better (but we should move OutOfRangeError into errors then). model is also fine for me.

Other than bikeshedding on the name, looks good.

@CodesInChaos
Copy link
Contributor Author

I don't think we should block this PR on the name bikeshedding, since it's already called model in the current master branch, this just cleans up the internal structure.


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

@tailhook
Copy link
Contributor

Yes. Let's continue discussion on #46

@tailhook tailhook merged commit eb212eb into edgedb:master Aug 19, 2020
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.

4 participants