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

Load FontAwesome from CDNJS #830

Merged
merged 2 commits into from Feb 28, 2019
Merged

Load FontAwesome from CDNJS #830

merged 2 commits into from Feb 28, 2019

Conversation

acdha
Copy link
Member

@acdha acdha commented Feb 28, 2019

Now that cdnjs/cdnjs#12307 is done we can finally complete #465 by consolidating all of our usage on a single service which uses the same CDN as the main site and also upgrades from 5.7.1 to 5.7.2.

This consolidates all of our usage on a single service which also uses
the CloudFlare infrastructure and upgrades from 5.7.1 to 5.7.2, which
finally completes #465.
@acdha acdha self-assigned this Feb 28, 2019
@acdha acdha requested review from klee72 and rstorey February 28, 2019 13:59
klee72
klee72 previously approved these changes Feb 28, 2019
Copy link
Collaborator

@klee72 klee72 left a comment

Choose a reason for hiding this comment

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

Just checked random pages and it's working great. However I noticed that we're using both 'fa' and 'fas' classes. Although 'fa' is still allowed in version 5, we'll probably want to just use 'fas' and 'fab' for future proofing, probably should be addressed in another ticket.

@acdha
Copy link
Member Author

acdha commented Feb 28, 2019

Agreed — I was also wondering whether we should be migrating to only loading the symbols we use or only loading SVG but that's more of a longer-term discussion about our front-end tools than something I want to tackle now.

This is the SHA-256 value from https://pypi.org/project/isort/#files for
the wheel package (the other value is the .tar.gz file) for the update
included in d7474f3
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.643% when pulling 28b39d9 on update-fontawesome-cdn into 5229350 on master.

Copy link
Member

@rstorey rstorey left a comment

Choose a reason for hiding this comment

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

This fixes the isort pipenv problem too.

@rstorey
Copy link
Member

rstorey commented Feb 28, 2019

Merging so I can get the isort fix into master

@rstorey rstorey merged commit 5b9ae23 into master Feb 28, 2019
@acdha acdha deleted the update-fontawesome-cdn branch February 28, 2019 23:57
@acdha
Copy link
Member Author

acdha commented Feb 28, 2019

Thanks - I was going to cherry-pick it but both changes are small.

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

4 participants