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 django bootstrap icon cache #419

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Conversation

frcroth
Copy link
Contributor

@frcroth frcroth commented Nov 4, 2023

Wow. For my local setup before this change the minimum page load time of the root page was abot 1200ms. With this change (after the first load) it is consistently between 200ms - 400ms. This is huge. 🚀

@frcroth frcroth requested a review from jeriox November 4, 2023 16:48
@coveralls
Copy link

coveralls commented Nov 4, 2023

Pull Request Test Coverage Report for Build 6756067503

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 74.538%

Totals Coverage Status
Change from base Build 6605019405: 0.02%
Covered Lines: 1250
Relevant Lines: 1677

💛 - Coveralls

@jeriox
Copy link
Contributor

jeriox commented Nov 6, 2023

@frcroth wow, amazing! I would be fine with approving this as a hotfix, but I think it would be nicer to download the icons once in the build step as we tried to avoid CDNs for bootstrap itself and i think it's weird that we then use a CDN for the icons.

@frcroth
Copy link
Contributor Author

frcroth commented Nov 6, 2023

Will you approve then? That is very low priority for me.

@jeriox
Copy link
Contributor

jeriox commented Nov 7, 2023

yes, but I would propose that you don't close the issue then so that we remember that there is something that can be done

@frcroth frcroth merged commit 3ef0cdd into fsr-de:main Nov 7, 2023
8 checks passed
@frcroth frcroth deleted the use-icon-cache branch November 7, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants