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

Use INSTALLED_APPS to look for unicorn components by default #211

Closed
wants to merge 0 commits into from

Conversation

adamghill
Copy link
Owner

@adamghill adamghill commented Jun 11, 2021

PR for #210.

  • Use INSTALLED_APPS for components by default
  • Cache the component look-up since they shouldn't move after they are found
  • Test that templates work correctly (or just default to unicorn as the sub-folder)
  • Update startunicorn to accept application name (should this default to "unicorn" if it's missing? or should it be required?)
  • Update documentation for creating new components, breaking changes, etc

@adamghill adamghill self-assigned this Jun 11, 2021
@nerdoc
Copy link
Contributor

nerdoc commented Jun 14, 2021

regarding the application name: I'd recommend keeping it as simple as possible. Keeping unicorn components in a separate app doesn't scale and adds more complexity. IF anyone wants to do that, nobody prevents him from doing so: create an empty app, create unicorn components in it and done. If you force the user (who doesn't want tnat) to create a unicorn app, or any "named" unicorn app, it's not a help, but a burden. So why not remove the setting completely? Alternatively, as help for beginners, make it mandatory for the startunicorn command: The dev has to name an app where the initial/example templates are placed in. You have to place them anywhere, so why not choose an app. Forcing him (or even encouraging him) to use a "unicorn" named app just adds unneeded complexity IMHO.

@nerdoc
Copy link
Contributor

nerdoc commented Jun 14, 2021

Oh, and keeping the unicorn components in a "unicorn" template subfolder is good IMHO. Others (reactor: unmaintained, etc.) do that too. Just keep in mind that, in a bigger project, it can get problematic, if two apps use a "counter" component. While normally they could be distinguished by foo/counter.html and bar/counter.html, in unicorn both would be named unicorn/counter.html. This could be solved in larges scaled projects by using unicorn/foo/counter.html - BUT: this adds complexity as well. you have to maintain an <app>/<component>.html structure beneath your templates/unicorn folder again. One solution would be to not look in templates folder, but create a separate structure as sibling to it: either "unicorn", or "components", as other projects do. Then you keep the components separate from normal templates in the first place.

@adamghill
Copy link
Owner Author

@nerdoc Yep, I agree with a lot of your points. I'm in the middle of updating the startunicorn command right now and should hopefully get this change released in the next few days.

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