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

Consider renaming Content, ContentSource, and ContentPayload #27

Open
idg10 opened this issue Jan 2, 2020 · 2 comments
Open

Consider renaming Content, ContentSource, and ContentPayload #27

idg10 opened this issue Jan 2, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@idg10
Copy link
Contributor

idg10 commented Jan 2, 2020

In discussion a PR at #19 (comment) @mwadams wrote:

ContentSource is a reference to a specific instance of content at a slug (e.g. version/state/however you are choosing to model it); it basically consists of {Slug,ID}.

Content is the common envelope for all content; it has timestamps, author info, title, etc.

ContentPayload is the actual "content" in the content, with a known but custom schema.

I find this pretty confusing. There's a class called Content but that's not the class we use to represent "the actual content".

I think we should try to think of more self-explanatory names while we still can.

Perhaps ContentSource should become ContentReference.

If Content is an envelope, but is not the actual content, maybe it should be ContentEnvelope. With those two changes, ContentPayload could probably keep its current name.

@mwadams
Copy link
Contributor

mwadams commented Jan 8, 2020

Content actually is the content though. You can happily have the content without a payload and it is still content (content that is just metadata). I think calling it an envelope was probably not conceptually helpful. It is a "base for all types of content, implemented via containment".

@mwadams
Copy link
Contributor

mwadams commented Jan 9, 2020

We have actually performed this renaming, and opted for ContentReference, Content and ContentPayload. This will be resolved in PR #19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants