Conversation
stsewd
left a comment
There was a problem hiding this comment.
I guess this can be updated and merged after https://github.com/readthedocs/sphinx_rtd_theme/pull/1064/files
|
This pull requests should actually be merged first, I based #1064 on the changes here. |
|
I guess the question is if we want to wait for a larger release or not |
|
Got it, I think we should include this together with the other in the same release |
|
@agjohnson is this change acceptable outside of the wyrm/bootstrap/sphinx2/html4/fontawesome5 changes? |
agjohnson
left a comment
There was a problem hiding this comment.
I don't think we need to be too careful here, but removing classes that downstream users might have been using should at very least be a breaking change probably.
| @@ -1,24 +0,0 @@ | |||
| .icon | |||
| @extend .fa | |||
There was a problem hiding this comment.
I maybe have slight concern here about removing the class, in case anyone downstream is using the .icon classes. This might be another backwards incompatible change
|
I'll target a 1.1 release for now, however I think this probably makes sense in a 2.0 release with a few other backwards incompatible changes before a 3.0 or greater bootstrap release. |
|
@agjohnson can this go into 1.1? |
|
Great update, should be merged quickly once we start working on 2.0. We could write alias css classes for |
This removes wyrm specific icon sass and migrates fully to using font awesome.
There should be no difference to users