search

jaynewey / django-octicons-v10

Django templatetags for GitHub Octicons v10.0.0+.
https://pypi.org/project/django-octicons-v10/
MIT License
2 stars 4 forks source link

Simplified code. #19

Closed apollo13 closed 2 years ago

apollo13 commented 2 years ago

Splitting by all dashes and then rejoining seems to be more work than needed :)

apollo13 commented 2 years ago

Looking further through the code: since self.icon_size seems to be only used as str() in the following code, the code could be simplified even more if we would accept that self.icon-size is a string, this would remove the need for converting to integer and back.

jaynewey commented 2 years ago

Splitting by all dashes and then rejoining seems to be more work than needed :)

Yep, good change 👍

Looking further through the code: since self.icon_size seems to be only used as str() in the following code, the code could be simplified even more if we would accept that self.icon-size is a string, this would remove the need for converting to integer and back.

Makes sense, however I think leaving it as an int is fine. It might be a breaking change to change to str because people might be getting the icon_size attribute and treating it as an int in their code.

apollo13 commented 2 years ago

Makes sense, however I think leaving it as an int is fine. It might be a breaking change to change to str because people might be getting the icon_size attribute and treating it as an int in their code.

Yeah, we could turn it into a icon_size property which would then turn it into int (and move the original into _icon_size, but I am not sure that is worth it.