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.

Sign up for GitHub

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

Jump to bottom

Add user-facing GZIP_CACHE_EXCLUDE_TYPES file type exclusion list #1288

Merged
merged 1 commit into from
Jan 7, 2021
Merged

Add user-facing GZIP_CACHE_EXCLUDE_TYPES file type exclusion list #1288

merged 1 commit into from
Jan 7, 2021

Conversation

sagamusix
Copy link
Contributor

@sagamusix sagamusix commented Aug 26, 2020 •
edited
Loading

Closes #852

@sagamusix sagamusix force-pushed the master branch 3 times, most recently from c87aefa to 9eaa52e Compare August 26, 2020 13:22
avaris
avaris reviewed Aug 28, 2020
View reviewed changes
gzip_cache/gzip_cache.py Outdated Show resolved Hide resolved
gzip_cache/gzip_cache.py Outdated Show resolved Hide resolved
gzip_cache/gzip_cache.py Outdated Show resolved Hide resolved
@avaris
Copy link
Member

avaris commented Aug 28, 2020

Also, needs documentation in the README for the new setting.

@sagamusix sagamusix force-pushed the master branch 2 times, most recently from d4460e5 to 9c1fb38 Compare August 28, 2020 14:55
@sagamusix
Copy link
Contributor Author

All requested changes have been applied and the README adjusted.

@sagamusix
Copy link
Contributor Author

Should I maybe convert EXCLUDE_TYPES and the new user setting from lists to tuples, so that they don't have to be converted to tuples inside should_compress again and again?

avaris
avaris approved these changes Aug 28, 2020
View reviewed changes
Copy link
Member

@avaris avaris left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@avaris
Copy link
Member

avaris commented Aug 28, 2020

Yes, those can be changed to tuples earlier. It's a minor thing and I didn't want to be nitpicky, but if you want to do it I'm not opposed :).

@sagamusix
Copy link
Contributor Author

Okay, converted to tuples as it wasn't really much extra work.

@sagamusix
Copy link
Contributor Author

Can we get this merged?

@justinmayer
Copy link
Member

Will have a look as soon as I can...

@sagamusix
Copy link
Contributor Author

Ping? :)

@justinmayer
Copy link
Member

Sincere apologies for the delay, @sagamusix. Many thanks for your work on this, and thanks to @avaris for reviewing! ✨

@justinmayer justinmayer merged commit 4a0e49e into getpelican:master Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avaris avaris avaris approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gzip_cache: Potential for a user-facing exclude option?
3 participants
@sagamusix @avaris @justinmayer