search

riboseinc / jekyll-theme-open-project

Jekyll theme for site suite presenting your open software and specifications.
MIT License
30 stars 3 forks source link

feat: update fa version and add variable for pro. closes #64 #65

Closed coder2000 closed 5 years ago

ronaldtse commented 5 years ago

@strogonoff for your review please. Thanks!

coder2000 commented 5 years ago

Updated the pro version to use the users desired version and integrity hash as it seems the hash is calculated for their own version.

ronaldtse commented 5 years ago

@strogonoff can you help review this? Thanks!

strogonoff commented 5 years ago

@strogonoff can you help review this? Thanks!

@ronaldtse see https://github.com/riboseinc/jekyll-theme-open-project/pull/65#discussion_r271612765. I’ll try to make this feature go in a bugfix release of the theme later this week.

To clarify, are we having someone wanting to use this open-source theme right now, or are we just trying to make it easier to adopt for potential future users?

ronaldtse commented 5 years ago

@strogonoff I don't personally know the author of this PR, so I assume he's trying to use this theme...

strogonoff commented 5 years ago

@strogonoff I don't personally know the author of this PR, so I assume he's trying to use this theme...

Understood, that’s what I meant. @coder2000 sorry that it’s not an immediate merge, just taking minimal measures to ensure your sites won’t appear broken in case free version doesn’t have the icons required by theme design.

coder2000 commented 5 years ago

I understand. Currently, I am using my own fork so the changes I have are working on my site. You designed it for your own needs and that is always where to start but as someone who is looking to use it as well, I am thinking about its flexibility.

You can see my work on my org page or my personal page. It's good to be cautious and make sure that changes don't break existing implementations, that's why there are tests.

ronaldtse commented 5 years ago

I agree that we should allowing using the free FA version, will merge this into a new branch for now. Thanks @coder2000 !