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

Fix #2812 — support ignoring assets in themes #2813

Merged
merged 1 commit into from Jun 3, 2017
Merged

Conversation

Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jun 2, 2017

This is #2812. cc @gwax (regarding making base smaller)

This implements the feature, without adding any ignores just yet. (baguettebox might be the first)

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska added this to the v7.8.7 milestone Jun 2, 2017
@Kwpolska Kwpolska requested a review from ralsina June 2, 2017 19:24
@gwax
Copy link
Contributor

gwax commented Jun 2, 2017

Ignoring individual assets interacts poorly with changes to an underlying theme; say a new asset is added to base. If the person adding the asset forgets to ignore it in a child theme, everyone that uses the child theme will start receiving the asset.

Of course, it's also potentially problematic if they don't. What happens if base adds an asset and requires it in a template. What if the child doesn't want the asset but hasn't overridden the template.

@gwax
Copy link
Contributor

gwax commented Jun 2, 2017

Fundamentally, we're fighting against a fragile base class problem.

@ralsina
Copy link
Member

ralsina commented Jun 2, 2017

@gwax

If base adds a new asset, and the child theme doesn't need it, it's on the child theme author to fix it, or some random asset gets published and 50kb get wasted.

I suppose we could just add a ** blacklist, where the child theme just says "give me no assets" and everything is manual from then on.

And yes, the other problem also exists, so changing the base theme can break the child in that case. OTOH, if the child theme blocks assets, then it's even more the author's task to handle that.

Honestly, I would block nothing and live with a few random extra assets not linked to anything in the output. Seems to me like a really minor thing to worry about :-)

Copy link
Member

@ralsina ralsina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@Kwpolska Kwpolska merged commit f45bafb into master Jun 3, 2017
@Kwpolska Kwpolska deleted the ignoring_assets branch June 3, 2017 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants