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

[EdgeDB] Create Producible, Film, EthnoArt, and Story schemas #2970

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

bryanjnelson
Copy link
Contributor

@bryanjnelson bryanjnelson commented Nov 22, 2023

@CarsonF - I'm not necessarily sold on the splitting out of all of these into their own esdl files now that I see the end result. A case could be made to place them all in the producible.esdl. Or perhaps even include them in the Producible module even? Thoughts appreciated.

Monday Task

┆Issue is synchronized with this Monday item by Unito

@bryanjnelson bryanjnelson force-pushed the edgedb-producibles-schema branch from 146e3a1 to c96862e Compare November 22, 2023 03:24
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not necessarily sold on the splitting out of all of these into their own esdl files now that I see the end result. A case could be made to place them all in the producible.esdl. Or perhaps even include them in the Producible module even? Thoughts appreciated.

Yeah I could go either way. I'd probably lean towards one file. Idk about moving them all into the Producible module. That would be a non-trivial name change compared to app code, but I definitely like the idea. Just not sure about how much we should be deviating at this point...

Speaking of deviation though, I wonder if we should just make Producible extend Named. My thought years ago was to not enforce this field, as producibles could be labeled differently, e.g. film title vs something else. This hasn't exactly played out well though. They all have a single name field right now, so it's kinda silly we don't just have it on the producible.
That certainly makes it easier:

abstract type Producible extending Resource, Mixin::Named {
  overloaded name {
    delegated constraint exclusive;
  }
  scriptureReferences: multirange<int32>;
}
type EthnoArt extending Producible;
type Film extending Producible;
type Story extending Producible;

dbschema/ethno-art.esdl Outdated Show resolved Hide resolved
dbschema/producible.esdl Outdated Show resolved Hide resolved
@bryanjnelson
Copy link
Contributor Author

Speaking of deviation though, I wonder if we should just make Producible extend Named. My thought years ago was to not enforce this field, as producibles could be labeled differently, e.g. film title vs something else. This hasn't exactly played out well though. They all have a single name field right now, so it's kinda silly we don't just have it on the producible.
That certainly makes it easier:

abstract type Producible extending Resource, Mixin::Named {
overloaded name {
delegated constraint exclusive;
}
scriptureReferences: multirange;
}
type EthnoArt extending Producible;
type Film extending Producible;
type Story extending Producible;

Yeah, I was wondering if you were going to say something about that. I like it...and will make it so!

@bryanjnelson bryanjnelson force-pushed the edgedb-producibles-schema branch from c96862e to 7e35aaa Compare November 22, 2023 17:52
@bryanjnelson bryanjnelson requested a review from CarsonF November 22, 2023 17:52
dbschema/producible.esdl Outdated Show resolved Hide resolved
@bryanjnelson bryanjnelson force-pushed the edgedb-producibles-schema branch from 7e35aaa to 55e314f Compare November 22, 2023 18:03
@bryanjnelson bryanjnelson requested a review from CarsonF November 22, 2023 18:03
@CarsonF CarsonF force-pushed the edgedb-producibles-schema branch from 55e314f to bcc7766 Compare November 28, 2023 22:33
@CarsonF CarsonF merged commit c64f7e8 into develop Nov 28, 2023
15 checks passed
@CarsonF CarsonF deleted the edgedb-producibles-schema branch November 28, 2023 22:38
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