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

Fixed targeting .NET 7.0 in ThemeController and ModuleDefinitionContr… #2895

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

vnetonline
Copy link
Contributor

…oller missed in #2888 and #2890

@vnetonline
Copy link
Contributor Author

The namespace for theme doesnt conform to the standard

image

should we make it [Owner].Themes.[Theme]?

@sbwalker sbwalker merged commit b21d202 into oqtane:dev Jun 14, 2023
@sbwalker
Copy link
Member

sbwalker commented Jun 14, 2023

I don't believe the framework should have used ".Themes" - it should have used ".Theme" but it's too much work to change the names of default themes at this point due to upgrade challenges. Including ".Theme" in the wwwroot is redundant currently as it's already within a Themes parent folder. However one item on the future roadmap is to transition to using the more standard .NET _content folder approach for static assets which would mean it would be useful to include the identifier in the folder name. And it would be useful for assembly names as well so that you can easily be able to differentiate themes from modules in the /bin. So I would be in favor of using [Owner].Theme.[Theme] as a naming convention.

@vnetonline
Copy link
Contributor Author

vnetonline commented Jun 14, 2023

Should I make that change to the template and submit a commit to this pull request ?

@sbwalker
Copy link
Member

Just submit a new PR for the change

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