search

sulu / SuluThemeBundle

MIT License
18 stars 15 forks source link

Use composer.json instead theme.json in docs #27

Closed plozmun closed 3 years ago

plozmun commented 3 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #issuenum
Related issues/PRs #issuenum
License MIT

What's in this PR?

SyliusThemeBundle doesn't use theme.json

niklasnatter commented 3 years ago

Thanks for the PR. Personally, I dont have a strong opinion on what name to use 🙂

@alexander-schranz @Prokyonn Do you remember if there was a reason for using theme.json instead of composer.json?

alexander-schranz commented 3 years ago

I would still go with the theme.json as I think the SyliusThemeBundle focus is on providing installable themes which are really need a composer.json and are packages registered in packagist. But this is not the main usage in sulu case and I think is missleading when a theme lives inside a project to have there composer.json inside a subfolder which not really have something there.

That's why its documented L73 with setting:

sylius_theme:
    sources:
        filesystem:
            filename: theme.json
plozmun commented 3 years ago

I would still go with the theme.json as I think the SyliusThemeBundle focus is on providing installable themes which are really need a composer.json and are packages registered in packagist. But this is not the main usage in sulu case and I think is missleading when a theme lives inside a project to have there composer.json inside a subfolder which not really have something there.

That's why its documented L73 with setting:

sylius_theme:
    sources:
        filesystem:
            filename: theme.json

Wops! my bad , i didn't see that in configuration, sorry