Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Conversation

oschaaf
Copy link
Member

@oschaaf oschaaf commented Feb 14, 2013

This allows templates to declare a list of unsupported pagespeed directives.
These will be filtered out, and a warning will be written for them.

@jeffkaufman
Copy link
Contributor

Mostly good, but I don't like the global variable hack. What if we moved the 'unsupported' check into nginx's write_pagespeed_open?

…ilable to other templates"

This reverts commit dc97039.
@oschaaf
Copy link
Member Author

oschaaf commented Feb 14, 2013

@jeffkaufman: "What if we moved the 'unsupported' check into nginx's write_pagespeed_open"

That is the way I tackled the problem initially, I have reverted the my last commit so you can judge it.
I thought, that adding a global might be worth it, as that would prevent every template from having to reimplement this filter functionality

@jeffkaufman
Copy link
Contributor

prevent every template from having to reimplemented this filter functionality

The code that would be duplicated across templates seems to be just:

if key.lower() in map(str.lower,pagespeed_unsupported):
    warnings.warn("%s not supported, skipping" % key)
    continue

we could pull the tricky part out into a function:

def check_unsupported(unsupported_directives, directive):
     return directive.lower() in map(str.lower, usupported_directives)

I could go either way on that. But I think the global-variable approach is too complicated and doesn't let "what does nginx support" be entirely up to nginx.conf.template.

@oschaaf
Copy link
Member Author

oschaaf commented Feb 14, 2013

@jeffkaufman
That makes sense to me

re: "I could go either way on that."
I think I am fine with it the way it is now.

jeffkaufman added a commit that referenced this pull request Feb 14, 2013
…orted-directives

generate config: don't filter unsupported pagespeed directives in .pyconf files
@jeffkaufman jeffkaufman merged commit 46e1e48 into oschaaf-generate-config Feb 14, 2013
@jeffkaufman
Copy link
Contributor

LGTM

@jeffkaufman jeffkaufman deleted the oschaaf-generate-config-unsupported-directives branch February 14, 2013 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants