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

Font assets not included in PHP package #186

Closed
bobmagicii opened this issue Feb 28, 2023 · 5 comments · Fixed by #192
Closed

Font assets not included in PHP package #186

bobmagicii opened this issue Feb 28, 2023 · 5 comments · Fixed by #192
Labels
bug Something isn't working

Comments

@bobmagicii
Copy link

is the composer package meant to include pre-compiled css and font files? the instructions here were pretty spartan and suggestive as such. it would be pretty great if so. or are we meant to go in there and deal with npm related stuff too? asking for a friend who is not a fan of front end and who is also me.

image

@mondeja mondeja closed this as not planned Won't fix, can't repro, duplicate, stale Mar 23, 2023
@bobmagicii
Copy link
Author

bobmagicii commented Mar 23, 2023

why have the composer package then? shrug

your readme will need updated because it suggests its one and done with composer. right now it says, install with composer, then see manual setup instructions. manual setup instructions say dl dist and link to css file. which does not exist in composer package. so why have the package if we are to just download the dist anyway. 🙃

@mondeja mondeja reopened this Mar 24, 2023
@mondeja
Copy link
Member

mondeja commented Mar 24, 2023

Sorry I haven't understood what you was trying to said.

You're right, the distribution folder is not generated before we "upload" to Packagist. I'm not sure how can we upload these assets to Packagist as they just release the tag pushed to Github, just supposing that we need to include the distribution folder in the tag which is... strange IMHO? Not sure. This problem can be checked downloading manually the latest version through this URL.

Unfortunately I don't have time to work on this nor motivation as I haven't used PHP in my whole life. This integration was added by a developer that is no longer part of the Simple Icons maintainers team. If someone want to bring a solution feel free to open a PR.

@mondeja mondeja added the bug Something isn't working label Mar 24, 2023
@mondeja mondeja changed the title the current PHP / Composer instructions Font assets not included in PHP package Mar 24, 2023
@bobmagicii
Copy link
Author

alright, i'll hit up my composer buddies for suggestions before i submit an idea with a sledgehammer that could have been a screwdriver.

@bobmagicii
Copy link
Author

bobmagicii commented Mar 30, 2023

after discussing with some peeps about what we're used to seeing in our corner of the eco system i got back three main ideas. none of these are demands just what i got back in my polling.

  1. my least fav - just not have the package. right now its a very goofy series of events where you would composer install and then download the release.zip. when you could just... download the release.zip. right now its got 37 downloads according to packagist.org, and at least 4 of them are me, so i don't forsee a swamping of issue tickets of people being like "what the hell" the rest of your life heheh.

  2. amend the readme with more verbose how to for the less js front end toolchain inclined like us backenders. how do we go from cd vendor/simple-icons/simple-icons-font to having a working css and font file in there, etc.

  3. my personal fav because it seems more common in my experience, so naturally the least convenient for you: change build practices such that you compile a dist dir, commit it, tag it, and you are done. no more ever uploading a release.zip, people just download the zip github will make and associate with that tag automatically. then the manual installers and the backend installers have everything they need. obviously i am not equipped to judge how this would mess with the npm normies. but yeah as a backend guy im pretty used to ninja'ing dist dirs from the repo and being done.

@mondeja
Copy link
Member

mondeja commented Apr 3, 2023

The solution is much more simpler but I don't have the motivation to do it testing it manually.

Just add a step to the publish workflow (after this step) to build the fonts before tagging the release, including the generated distribution folder (dist/) on the tag. After that, the fonts will be included in the Packagist package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants