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

Remove OrchardCore.Contents.FileContentDefinition usage from remaining recipes #11200

Closed
Piedone opened this issue Feb 17, 2022 · 3 comments · Fixed by #11210
Closed

Remove OrchardCore.Contents.FileContentDefinition usage from remaining recipes #11200

Piedone opened this issue Feb 17, 2022 · 3 comments · Fixed by #11210

Comments

@Piedone
Copy link
Member

Piedone commented Feb 17, 2022

Is your feature request related to a problem? Please describe.

Under #10799 the Agency and Blog recipes got OrchardCore.Contents.FileContentDefinition usage removed, since as was the consensus, it's only useful in specific cases and most of the time you need to turn it off. The same issue is there with the Headless and Blank recipes. Especially for the Blank recipe I see no reason to include anything apart from the bare bones. (And if you're using and SQL DB, which unless you're really savvy you will do, I don't see the point of OrchardCore.Contents.FileContentDefinition at all: Why make data storage more complicated?)

Describe the solution you'd like

Just remove the OrchardCore.Contents.FileContentDefinition enable step from the two recipes.

Describe alternatives you've considered

The alternative is turning it off after every single setup and migrate content definition as described in the docs.

@rjpowers10
Copy link
Contributor

rjpowers10 commented Feb 17, 2022

(And if you're using and SQL DB, which unless you're really savvy you will do, I don't see the point of OrchardCore.Contents.FileContentDefinition at all: Why make data storage more complicated?)

FWIW this feature burned us a bit in the past. When we were first starting out we enabled it since at the time the Blog recipe was using it and so we just followed that not really understanding what it did. Now, I'm sure this next part was our own fault, but we ended up in a situation where we had content items in the DB that didn't have matching content type definitions in this file, and the error we got was rather cryptic at the time (not sure if the error was made more clear later on). I think it was because we were copying one site to another, and we copied the DB without knowing we had to copy that file as well.

Anyway, for whatever it's worth I personally found the feature more confusing than helpful.

@sebastienros
Copy link
Member

Fine then!

@sebastienros sebastienros added this to the 1.x milestone Feb 17, 2022
@Skrypt
Copy link
Contributor

Skrypt commented Feb 18, 2022

My bad I thought I did not look closely enough when I did that PR. I agree with removing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants